Skip to content

[driver] Document what a "subcommand" is (NFC) #11940

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

Closed
wants to merge 1 commit into from

Conversation

modocache
Copy link
Contributor

There are a few references to "subcommands" in the repository, but no explanation of what they are. Beef up the docblock for shouldRunAsSubcommand() to make things a little clearer. Specifically, I'd like to make the difference between driver jobs and driver subcomnmands clearer.

In addition, remove a double negative comment, "If we are not run as 'swift', don't do anything special", and replace it with something a little clearer (in my opinion).

There are a few references to "subcommands" in the repository, but no
explanation of what they are. Beef up the docblock for
`shouldRunAsSubcommand()` to make things a little clearer.
Specifically, I'd like to make the difference between driver jobs and
driver subcomnmands clearer.

In addition, remove a double negative comment, "If we are not run as
'swift', don't do anything special", and replace it with something a
little clearer (in my opinion).
@CodaFi CodaFi requested a review from jrose-apple September 18, 2017 04:18
@@ -61,7 +61,10 @@ extern int modulewrap_main(ArrayRef<const char *> Args, const char *Argv0,
extern int swift_format_main(ArrayRef<const char *> Args, const char *Argv0,
void *MainAddr);

/// Determine if the given invocation should run as a subcommand.
/// Determine if the given invocation should run as a "subcommand".
/// Examples of "subcommands" are 'swift build' or 'swift-test', which are
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: please include a newline here in case anyone ever does run Doxygen on the Swift source.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also that should be "swift test", right?

// We only recognize subcommands in the form of 'swift <name-of-subcommand>'.
// If we are being run as an executable named something other than 'swift',
// we do not run a subcommand. This means we do not recognize symlinks with
// alternate names, for example 'swiftc build', as subcommands.
Copy link
Contributor

Choose a reason for hiding this comment

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

That's not what the original text was getting at. We do want to distinguish "swift" vs. "swiftc", but ideally we wouldn't distinguish "swift" vs. "swift-4.1" (or similar).

@CodaFi
Copy link
Contributor

CodaFi commented Apr 10, 2020

I’ve integrated this PR and responded to Jordan’s feedback in the linked PR.

Thanks y’all.

@CodaFi CodaFi closed this Apr 10, 2020
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.

3 participants