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

Streamline some goal descriptions. #10911

Merged
merged 1 commit into from Oct 5, 2020

Conversation

benjyw
Copy link
Sponsor Contributor

@benjyw benjyw commented Oct 5, 2020

The old ones contained unnecessary detail.

[ci skip-rust]

[ci skip-build-wheels]

The old ones contained unnecessary detail.

[ci skip-rust]

[ci skip-build-wheels]
@@ -31,7 +31,7 @@


class TargetTypesSubsystem(LineOriented, GoalSubsystem):
"""List all the registered target types, including custom plugin types."""
"""List all registered target types."""
Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

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

Plugins are the basic way Pants works, there's no need to qualify. This might make the reader think "hm, where are plugins not included"?

@@ -60,7 +60,7 @@ def generate_exe(self, _: Platform) -> str:


class CountLinesOfCodeSubsystem(GoalSubsystem):
"""Count lines of code using `scc` (Succinct Code Counter, https://github.com/boyter/scc)."""
"""Count lines of code."""
Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

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

Too much detail.

@@ -29,7 +29,7 @@ class BuiltPackage:


class PackageSubsystem(GoalSubsystem):
"""Package an asset and put in `--distdir`, such as an archive, PEX, wheel, AWS Lambda, etc."""
"""Create a distributable package."""
Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

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

The examples are unnecessary, and --distdir mention is too much detail.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 0.0% when pulling d63b2f7 on benjyw:streamline_goal_description into 7fc06c3 on pantsbuild:master.

@benjyw benjyw merged commit 0e97c3c into pantsbuild:master Oct 5, 2020
Copy link
Sponsor 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.

These changes all look good.

But because these descriptions end up in ./pants help and on https://www.pantsbuild.org/docs/reference-all-goals (where having more information is very, very helpful), we might want to differentiate the "first sentence" of a description from the rest, and then significantly expand the rest of the description to add more meat to each of the pages.

For example, the generated package page now looks like https://www.pantsbuild.org/docs/reference-package ... which really doesn't do enough to explain what the goal does. On the other hand, the hand written package page goes into a lot more depth: https://www.pantsbuild.org/docs/python-package-goal ... some of which is generic, and could be in here.

@benjyw benjyw deleted the streamline_goal_description branch July 7, 2021 00:12
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

4 participants