Skip to content

Remove duplicate sccache flag #75390

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

Merged
merged 1 commit into from
Jul 24, 2024
Merged

Remove duplicate sccache flag #75390

merged 1 commit into from
Jul 24, 2024

Conversation

mattmassicotte
Copy link
Contributor

Hello!

I'm trying to build the compiler, following along with the Getting Started guide. I noticed that the guide specifically suggests adding the --sccache flag if needed, but it is included in the example invocation.

If you installed and want to use Sccache, add --sccache to the invocation.

I seems like the intention here was to omit it, and explain how to include it if desired.

Copy link
Collaborator

@AnthonyLatsis AnthonyLatsis left a comment

Choose a reason for hiding this comment

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

Thanks.

@AnthonyLatsis
Copy link
Collaborator

Please make the commit message more descriptive. Imagine someone else reading it:

  • Where was the change made? Could the change have possibly broken anything? A [docs] or docs: prefix is a good enough indication.
  • I think "redundant" is more apposite than "duplicate". The latter is normally understood to imply the existence of an exact copy.
  • I would spell out the flag as is for clarity: --sccache.
  • Why was the flag redundant? The answer is not obvious even if you read the diff. This is a question for the commit description.

@xedin
Copy link
Contributor

xedin commented Jul 23, 2024

I can fix commit message on squash this time.

@xedin
Copy link
Contributor

xedin commented Jul 23, 2024

@swift-ci please smoke test

@xedin
Copy link
Contributor

xedin commented Jul 23, 2024

@swift-ci please smoke test Linux platform

@xedin xedin merged commit 721f529 into swiftlang:main Jul 24, 2024
3 checks passed
@xedin
Copy link
Contributor

xedin commented Jul 24, 2024

@mattmassicotte Thank you for the contribution!

@mattmassicotte
Copy link
Contributor Author

Thank you both for accepting this and fixing it up. I just used the in-browser GitHub edit facility to throw this together. The commit message was terrible and, in hindsight, not a mistake I should have made. Will be more careful in the future.

@mattmassicotte mattmassicotte deleted the patch-1 branch July 25, 2024 15:51
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