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

[docs] Add First Pull Request guide and Getting Started guide. #33786

Conversation

varungandhi-apple
Copy link
Contributor

This improves upon the existing documentation to provide a clearer end-to-end
workflow for new contributors and people who wish to build the toolchain
locally but do not intend to submit patches.

We also provide more directions for systematically utilizing our existing
documentation.

@varungandhi-apple
Copy link
Contributor Author

If you want to see the rendered documentation, you can either click "Display Rich Diff" in the top right or the triple-dots and then click "View File".

@varungandhi-apple
Copy link
Contributor Author

About the couple of TODOs left in the FAQ -- those are intentional. I'm hoping those can serve as bite-sized tasks for other people (including new contributors but not necessarily) to contribute to the documentation without worrying about setting up the scaffolding around it.

Copy link
Contributor

@hamishknight hamishknight 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 awesome!

@bnbarham
Copy link
Contributor

bnbarham commented Sep 3, 2020

Since I can't add a comment to unchanged lines, in README.md:

  • A number of the links under "Welcome to Swift" no longer work (probably just want to remove them, seems like that is a TOC section
  • In "Build Failures" the "correct release" link to "#macos" also doesn't work any more

Otherwise I think the changes look really good - I've only been here about a month now and these definitely would have helped back when I started :)!

@varungandhi-apple
Copy link
Contributor Author

varungandhi-apple commented Sep 4, 2020

Leaving this up for one more day until end-of-Monday in case anyone has further suggestions. (Will clean up commit history after addressing all the feedback.)

There are still 1-2 unresolved points. Since Monday is a holiday in the US, I will be landing this likely by end-of-day Tuesday (Sep 8) or Wednesday (Sep 9).

Copy link
Member

@Jumhyn Jumhyn 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 excellent @varungandhi-apple! Super excited to see these docs get added.

@benrimmington
Copy link
Contributor

Should the /docs/ABI/ files be listed in the /docs/README.md index?

@varungandhi-apple
Copy link
Contributor Author

varungandhi-apple commented Sep 4, 2020

It was broken up in 7c71b32 but docs/README.md wasn't updated. We can fix that in a separate PR, since it's not related to the new guides.

This improves upon the existing documentation to provide a clearer end-to-end
workflow for new contributors and people who wish to build the toolchain
locally but do not intend to submit patches.

We also provide more directions for systematically utilizing our existing
documentation.
@varungandhi-apple varungandhi-apple force-pushed the vg-doc-new-starter-guides branch from f6974a3 to ae8db93 Compare September 9, 2020 03:28
@varungandhi-apple
Copy link
Contributor Author

@swift-ci please smoke test and merge

@benrimmington

This comment has been minimized.

@benrimmington
Copy link
Contributor

@swift-ci Please smoke test Linux platform

@varungandhi-apple varungandhi-apple merged commit d50bc21 into swiftlang:master Sep 9, 2020
@varungandhi-apple varungandhi-apple deleted the vg-doc-new-starter-guides branch September 9, 2020 15:43
Comment on lines +19 to +20
- Use `build-script`'s various `--skip-*` flags to skip configuring for
platforms that you do not care about.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be mentioned that not all --skip-* flags are documented in --help?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you have some specific flags in mind here? I think the right step here would be to document the flags...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I was under the impression not all skip flags were shown. I don't have a long list or anything, but the ones I have in my shell history are specific to testing, which may void my suggestion (--skip-test-cmark and --skip-test-swift).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All flags should be documented IMO, regardless of whether they are specific to testing or not.

Copy link
Contributor

Choose a reason for hiding this comment

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

agreed

Comment on lines +67 to +69
- Via SSH (recommended):
If you plan on contributing regularly, cloning over SSH provides a better
experience. After you've [uploaded your SSH keys to GitHub][]:
Copy link
Contributor

Choose a reason for hiding this comment

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

why is ssh recommended? what experiences are better?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IIRC you need to repeatedly enter your GitHub password when using HTTPS to clone the different repositories and I also think you need to enter it when pushing changes. Is the suggestion that I explicitly spell out the benefit of using SSH over HTTPS?

Copy link
Contributor

@kastiglione kastiglione Sep 9, 2020

Choose a reason for hiding this comment

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

I use https and the GitHub token gets saved in my login keychain. I only ever enter it once for all of GitHub, no additional use for clones or pushes. For macOS, I think this is as convenient or more convenient than ssh.

In the past, GitHub staff have told me https gets better performance.

I didn't have a suggestion, but now I would suggest removing the recommendation and letting people pick.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ended up looking at this a bit more and turns out:

  1. GitHub recommends using HTTPS over SSH.
  2. Credentials aren't required for cloning public repositories but are required for pushing, which may get annoying... (tested this on Linux).

I'm kinda' in two minds on what to do about this. My general tendency when writing documentation for a beginner (in this case, someone beginning to contribute) to:

  1. If one option "just works", don't suggest multiple options. For example, I suggest using sccache instead of having a paragraph dedicated to "you could use sccache or you could use cacche, here's how you setup either, here's why you might want one or the other etc."
  2. If there is a good reason to suggest multiple options, recommend one option, and briefly describe why they would prefer one over the other.

This is intended to reduce the burden of decision-making; generally the person who is getting started is in a worse position to make that decision than the person writing the documentation, so we make the decision on their behalf.


Maybe I should put HTTPS first with a "(recommended by GitHub)" and point out that you may need to enter credentials when pushing changes, which can be avoided using SSH?

Copy link
Contributor

Choose a reason for hiding this comment

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

My general tendency when writing documentation for a beginner (in this case, someone beginning to contribute)

beginner to contribute to GitHub in general, or Swift in specific? If GitHub in general, maybe there's some existing documentation that compares the two alternatives, and then we can point them there. If focused on a begging contributor to Swift, we sidestep and say to use whichever protocol you typically use.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It could be both. Particularly thinking about students here. Generally, I've seen students have some familiarity with GitHub, but that's not always the case. That's why I've tried to be explicit about the git/GitHub-related steps instead of assuming that someone already knows the typical GitHub workflow.

Also, someone experienced might be used to workflows with other tools like Phabricator or Perforce, not so much with GitHub.


If you used `--xcode` earlier, you will see an Xcode project generated under
`../build/Xcode-RelWithDebInfoAssert/swift-macosx-x86_64`. When you open the
project, Xcode might helpfully suggest "Automatically Create Schemes". Most of
Copy link
Contributor

@kastiglione kastiglione Sep 9, 2020

Choose a reason for hiding this comment

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

If desired, we could make build-script update the project to disable this feature for the generated project.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PRs welcome! 😛

Copy link
Contributor

Choose a reason for hiding this comment

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

I took a quick look, I don't see any obvious place to perform "post cmake generate actions". Is there such a place? Fwiw, the fix is to run:

plutil -replace IDEWorkspaceSharedSettings_AutocreateContextsIfNeeded -bool false \
    <Project>.xcodeproj/project.xcworkspace/xcshareddata/WorkspaceSettings.xcsettings

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know; I'm not really familiar with this part of the code.

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.