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

Rename @console_rule to @goal_rule #8942

Merged
merged 1 commit into from
Jan 10, 2020

Conversation

Eric-Arellano
Copy link
Contributor

@Eric-Arellano Eric-Arellano commented Jan 10, 2020

Any time that you add a new top-level goal in V2—that is a "verb" for Pants like test, fmt, setup-py, and dependencies—you must define the same three basic things:

Goal, GoalSubsystem, @console_rule

One of those does not fit with the other two. In contrast, these three all belong:

Goal, GoalSubsystem, @goal_rule

What is an @console_rule?

There are (at least) 3 ways of conceptualizing what an @console_rule means:

  1. A rule that has exclusive access to the Console.
    • Only @console_rules can write to stdout and stderr.
    • This property is represented in the name @console_rule.
  2. A rule that is not cached.
  3. A rule that maps 1-1 with a top-level goal / returns an engine.goal.Goal.

All three of these properties are true and important components of an @console_rule. However, I argue that #3 is the most important: fundamentally, an @console_rule is a special rule that hooks up to a Pants goal.

Right now, the name @console_rule optimizes for property #1. In contrast, @goal_rule focuses on property #3.

(NB: all three of these properties will continue applying regardless of the name)

Leveraging prior Pants knowledge

Most Pants users are familiar with the idea of a goal. It's the very first concept introduced in our docs and every single Pants run involves invoking goals, regardless of V1 vs. V2.

In contrast, fewer users are familiar with the Console abstraction, which is never mentioned in our docs and is more of an implementation detail for how V2 Pants works.

We do expect users to soon write their own rules in plugins. The name @goal_rule will allow them to leverage their prior knowledge, just as subsystem_rule leverages prior knowledge better than optionable_rule did.

Copy link
Contributor

@pierrechevalier83 pierrechevalier83 left a comment

Choose a reason for hiding this comment

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

A little bit of a heavy-handed change, so I would like to see many approvals before this got merged; but I agree with the reasoning and I think this change would help for teachability of the pants concepts to new contributors 😄

@herman5
Copy link
Member

herman5 commented Jan 10, 2020

Good stuff, Eric! I’m onboard with @goal_rule for exactly what you outlined:

Goal, GoalSubsystem, @console_rule - One of those does not fit with the other three

Given that I’m still new to Pants V2, the @goal_rule convention fits easily into my mental model along with being easy to remember (@console_rule actually requiring effort to recall as the naming is more orthogonal).

Copy link
Contributor

@codealchemy codealchemy left a comment

Choose a reason for hiding this comment

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

👍 👍 (for the same reasons you and others have already touched on)

Copy link
Contributor

@cosmicexplorer cosmicexplorer left a comment

Choose a reason for hiding this comment

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

This is a really well-thought-out argument. Thank you for taking the time to describe it so clearly!

Copy link
Member

@stuhood stuhood left a comment

Choose a reason for hiding this comment

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

Works for me.

Not to throw a wrench in the works, but: did you consider just @goal...? I'd be fine with either

@gshuflin
Copy link
Contributor

Works for me.

Not to throw a wrench in the works, but: did you consider just @goal...? I'd be fine with either

I'd vote for @goal_rule over @goal to make it clearer that a @goal_rule is still a type of @rule

@Eric-Arellano
Copy link
Contributor Author

I'd vote for @goal_rule over @goal to make it clearer that a @goal_rule is still a type of @rule

Plus one. I also find the difference between Goal and @goal slightly confusing.

Thanks for the reviews, everyone! Will merge once this goes green.

@Eric-Arellano Eric-Arellano merged commit af4a304 into pantsbuild:master Jan 10, 2020
@Eric-Arellano Eric-Arellano deleted the goal-rule branch January 10, 2020 21:25
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.

7 participants