Skip to content

Resolves #75109 #75163

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 9 commits into from
Closed

Resolves #75109 #75163

wants to merge 9 commits into from

Conversation

vidhupv
Copy link
Contributor

@vidhupv vidhupv commented Jul 11, 2024

New commits include uploading the new screenshot to docs under the name SwiftGithubCreatePRScreenshot.png and also changing the name of the screenshot location in the HowTo Guide for "First Pull Requests"

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.

  • Please take the screenshot directly from https://github.com/swiftlang/swift, which is where the doc suggests us to navigate. You could even crop it somewhat like this without the org name that brought about this issue:

    Screenshot 2024-07-12 at 06 09 04
  • The rest of our screenshots are in light mode. Let’s be consistent about that.

  • Mind tidying up the commit history and giving this pull request a proper title?

@vidhupv
Copy link
Contributor Author

vidhupv commented Jul 12, 2024

@AnthonyLatsis I shall do that. Just a couple of questions:

  1. Should I do it exactly the way you sent or should I include the swiftlang/swift portion at the top of the page as well considering how that was in the current screenshot.
  2. Should I convert this pull request to draft in order to tidying the commit history?

@AnthonyLatsis
Copy link
Collaborator

  1. What do you think?
  2. No need. The draft state is to indicate that changes are not ready for review. For example, reviews are not requested until you undraft if you start off with a draft PR.

@AnthonyLatsis
Copy link
Collaborator

Could you try tidying up your commit history by squashing your commits via an interactive rebase rather than spawning new PRs? Let me know if you need help with that. You will definitely benefit from this skill if you are planning to contribute more. Also, we prefer rebasing on main over merging with main when resolving conflicts — these intermediary merge commits pollute the commit history for no good reason.

@vidhupv
Copy link
Contributor Author

vidhupv commented Jul 13, 2024

Hey, I would greatly appreciate any help on that. I am just learning to use the rebase editor on my terminal. I have dropped a few previous redundant commits and am in the process of squashing the valid ones right now.

@AnthonyLatsis
Copy link
Collaborator

Hey, I would greatly appreciate any help on that.

Sure, feel free to reach out if you get stuck with anything specific.

@AnthonyLatsis
Copy link
Collaborator

Superseded by #75220.

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.

Replace old screenshot with new one in HowToGuide for First Pull
2 participants