Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tweak version linter max_depth #14386

Merged
merged 2 commits into from
Oct 19, 2023
Merged

Conversation

psteckler
Copy link
Member

Lower the version linter max_depth to 9 (was 12). That's the depth of the type shapes, as S-expressions, used when comparing them across branches. The type shapes are truncated to that depth, so that differences in contained types are elided, avoiding false positives.

Tested by running the script locally with edited type shape files. Changes tried:

  • change to built-in type (should give error)
  • change to variant constructor (should give error)
  • change to record field name (should give error)
  • change to record field name in contained type (should not give error)

A depth of 10 gave an error for the last case. Backing off to 9 removed the generated error.

There may still be occasional false positives. For example, a contained type might be simpler than a record, such as a built-in type or variant, for instance. Those should be infrequent, and easy enough to spot, because the change to the contained type will also generate an error.

In fact, the truncation code wasn't working, because the type shape was not converted to an S-expression. Fixed here.

Added the --no-clobber option to wget, so that the type shape file is not downloaded if it exists (which it did with repeated local testing).

Closes #14232.

@psteckler psteckler requested review from a team as code owners October 18, 2023 19:33
@psteckler
Copy link
Member Author

!ci-build-me

@psteckler
Copy link
Member Author

!ci-build-me

@mrmr1993
Copy link
Member

!approved-for-mainnet

@psteckler psteckler merged commit f60aaa8 into berkeley Oct 19, 2023
@psteckler psteckler deleted the feature/tweak-linter-max-depth branch October 19, 2023 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants