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

Add new data class for rule transition factories. #13749

Closed
wants to merge 2 commits into from

Conversation

katre
Copy link
Member

@katre katre commented Jul 27, 2021

Part of the fix for #13751.

@katre katre requested a review from gregestren July 27, 2021 13:54
@katre katre requested a review from lberki as a code owner July 27, 2021 13:54
@google-cla google-cla bot added the cla: yes label Jul 27, 2021
@gregestren
Copy link
Contributor

What does this change facilitate?

@katre
Copy link
Member Author

katre commented Jul 27, 2021

Sorry, created an issue in github. This is part of addressing the crash when a user tries to use an attribute-only transition (like the exec transition) directly on a rule. See #13751.

@gregestren
Copy link
Contributor

That makes more sense, thanks. Does something in this PR fix the crash, or is there more to come?

I'm being more pedantic than normal since we've noticed we have a lot of abstractions for how transitions are created and applied. Since this adds another abstraction, I want to ensure I understand clearly its role.

@katre
Copy link
Member Author

katre commented Jul 27, 2021

The next commit adds a marker interface so that TransitionFactory can only be created with specified data classes (AttributeTransitionData and the new RuleTransitionData). The final commit adds a type enum and an error message when the wrong type is used.

@bazel-io bazel-io closed this in 525227e Jul 27, 2021
@katre katre deleted the ttf-01-rule-transition-data branch July 27, 2021 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants