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

Make GoalSubsystem subclass Subsystem. #10483

Merged
merged 2 commits into from Jul 28, 2020

Conversation

benjyw
Copy link
Sponsor Contributor

@benjyw benjyw commented Jul 28, 2020

Previously it subclassed the same bases as Subsystem,
and reimplemented some of its functionality, which
was a weird quirk: you'd expect something called GoalSubsystem,
which acts like a Subsystem, to actually be one.

This change folds one of those bases (SubsystemClientMixin)
into Subsystem, and gets rid of it.

A future change will similarly merge Subsystem and Optionable,
since there is now only one kind of Optionable.

[ci skip-rust]

[ci skip-build-wheels]

[ci skip-rust]

[ci skip-build-wheels]
@coveralls
Copy link

coveralls commented Jul 28, 2020

Coverage Status

Coverage remained the same at 0.0% when pulling 17aea0c on benjyw:goal_subsystem_is_subsystem into ae4c1bd on pantsbuild:master.

Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Can SubsystemClientMixin be deleted now?

[ci skip-rust]

[ci skip-build-wheels]
@benjyw benjyw force-pushed the goal_subsystem_is_subsystem branch from 9c78cd7 to 17aea0c Compare July 28, 2020 05:36
@benjyw
Copy link
Sponsor Contributor Author

benjyw commented Jul 28, 2020

Awesome! Can SubsystemClientMixin be deleted now?

It was supposed to have been, I think I didn't push that part.

@benjyw benjyw merged commit afae657 into pantsbuild:master Jul 28, 2020
@benjyw benjyw deleted the goal_subsystem_is_subsystem branch July 28, 2020 15:38
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.

None yet

3 participants