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.

Sign up for GitHub

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

Jump to bottom

Check invariant during Starlark options parsing #19003

Closed
wants to merge 3 commits into from
Closed

Check invariant during Starlark options parsing #19003

wants to merge 3 commits into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Jul 20, 2023 •
edited
Loading

The particular implementation of BuildSettingLoader always loads targets synchronously without every returning null, so the return value of parse is expected to be true. Check this invariant to make these calls easier to understand.

@fmeum fmeum changed the title TMP: Test that all Starlark options are loaded Check that all Starlark options are loaded during option parsing Jul 20, 2023
@fmeum fmeum changed the title Check that all Starlark options are loaded during option parsing Check invariant during Starlark options parsing Jul 20, 2023
@fmeum fmeum marked this pull request as ready for review July 20, 2023 10:36
@fmeum fmeum requested a review from gregestren July 20, 2023 10:36
@github-actions github-actions bot added the awaiting-review PR is awaiting review from an assigned reviewer label Jul 20, 2023
@gregestren gregestren added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally team-Configurability platforms, toolchains, cquery, select(), config transitions and removed awaiting-review PR is awaiting review from an assigned reviewer labels Jul 20, 2023
@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Jul 21, 2023
@fmeum fmeum deleted the ensure-options-parsed branch July 22, 2023 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gregestren gregestren gregestren approved these changes

Assignees
No one assigned
Labels
team-Configurability platforms, toolchains, cquery, select(), config transitions
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fmeum @gregestren @iancha1992