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.

Sign up for GitHub

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

Jump to bottom

CONTRIBUTING: Added instructions for PR workflow #1105

Merged
merged 16 commits into from
Apr 3, 2023
Merged

CONTRIBUTING: Added instructions for PR workflow #1105

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
72acc00
CONTRIBUTING: Added instructions for PR workflow
chrysle Feb 25, 2023
6a1c089
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 25, 2023
05763ad
Added changelog entry
chrysle Feb 25, 2023
92b896e
Merge branch 'contributing-pr-workflow' of github.com:chrysle/attrs i…
chrysle Feb 25, 2023
8d636a9
Use inline code for changelog filename
chrysle Feb 26, 2023
1977bda
Add console prompt to command
chrysle Feb 26, 2023
aa3f8b8
Made some more improvements
chrysle Feb 26, 2023
79c2f69
Merge branch 'main' into contributing-pr-workflow
chrysle Feb 27, 2023
e968bcc
Merge branch 'main' into contributing-pr-workflow
hynek Apr 3, 2023
810b8d6
Apply suggestions from code review
hynek Apr 3, 2023
43d0945
Update .github/CONTRIBUTING.md
hynek Apr 3, 2023
6f8d73e
Update changelog.d/1105.change.md
hynek Apr 3, 2023
6b3e5f6
Merge branch 'main' into contributing-pr-workflow
hynek Apr 3, 2023
75a41aa
Update .github/CONTRIBUTING.md
hynek Apr 3, 2023
da16588
Update .github/CONTRIBUTING.md
hynek Apr 3, 2023
8e822fd
Update .github/CONTRIBUTING.md
hynek Apr 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 35 additions & 3 deletions .github/CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,36 @@ You can (and should) run our test suite using [*tox*].
However, you’ll probably want a more traditional environment as well.
We highly recommend to develop using the latest Python release because we try to take advantage of modern features whenever possible.

Clone the *attrs* repository to your computer:
First [fork](https://github.com/python-attrs/attrs/fork) the repository on GitHub.
hynek marked this conversation as resolved.
Show resolved Hide resolved

Clone the fork to your computer:

```console
$ git clone git@github.com:python-attrs/attrs.git
$ git clone git@github.com:<your-username>/attrs.git
```

Or if you prefer to use Git via HTTPS:

```console
$ git clone https://github.com/python-attrs/attrs.git
$ git clone https://github.com/<your-username>/attrs.git
```

Then add the *attrs* repository as *upstream* remote:

```console
$ git remote add upstream git@github.com:python-attrs/attrs.git
hynek marked this conversation as resolved.
Show resolved Hide resolved
```

You can of course also rename the *origin* remote (your fork) to *fork* and add the repository as *origin*.
hynek marked this conversation as resolved.
Show resolved Hide resolved
hynek marked this conversation as resolved.
Show resolved Hide resolved

The next step is to sync the upstream repository with your local copy:

```console
$ git fetch upstream
chrysle marked this conversation as resolved.
Show resolved Hide resolved
```

This is important to obtain eventually missing tags, which are needed to install the development version later on. See [#1104](https://github.com/python-attrs/attrs/issues/1104) for more information.
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit inaccurate, though. It fetches both branches and tags. But not necessarily all.
Maybe, it makes sense to add --tags --prune-tags. Maybe, it makes sense to specify the branch name too.
git fetch upstream main --tags --prune-tags would probably represent what we want here.

Copy link
Member

Choose a reason for hiding this comment

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

With the comment in another thread above, it could probably be just git fetch upstream --prune-tags.

Copy link
Member

Choose a reason for hiding this comment

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

Do we need --prune-tags for anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will remove any local tags that do no longer exist in the remote; I guess it's useful.
I think the --prune option needs to be passed too for this to succeed? See git fetch documentation.

Copy link
Member

Choose a reason for hiding this comment

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

yeah that seems too specific for a "get up and contribute real quick 101" for a project that never had to remove a tag. I really want to be helpful, but if we make it too complicated, people won't read / understand it. It's a tedious balance act. :-/

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, tags are supposed to be immutable, anyway. Also, even if tags change, it probably won't upset the contributors since they shouldn't care too much when the generated version is a little off. It's not like they are going to make official dists from that env, anyway. Though, I'd like https://github.com/python-attrs/attrs/pull/1105/files#r1146115629 to be accepted because it'll set up the local config of the Git repo to always fetch the right things w/o extra args.

hynek marked this conversation as resolved.
Show resolved Hide resolved

Change into the newly created directory and after activating a virtual environment install an editable version of *attrs* along with its tests and docs requirements:

```console
Expand All @@ -81,6 +99,20 @@ $ make html

The built documentation can then be found in `docs/_build/html/`.

To file a pull request, create a new branch on top of the upstream repository:

```console
$ git checkout -b my_topical_branch upstream/main
hynek marked this conversation as resolved.
Show resolved Hide resolved
hynek marked this conversation as resolved.
Show resolved Hide resolved
```

Make your changes, push them to your fork (the remote *origin*) and publish the PR!
Copy link
Member

Choose a reason for hiding this comment

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

This could be accompanied with the following snippet:

$ git push -u origin feature/my-topic-branch

-u marks the branch on the origin remote (aka fork) as corresponding to the local one so that git push w/o args would pick it up automatically.

Copy link
Member

Choose a reason for hiding this comment

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

i removed the branch name because it feels tedious and it's unclear to me what problem it's supposed to solve. happy to be corrected, but I'll try to keep it as simple as possible.

hynek marked this conversation as resolved.
Show resolved Hide resolved

Before starting to work on your next pull request, run the following command to sync your local repository with the remotes:
hynek marked this conversation as resolved.
Show resolved Hide resolved

```console
$ git fetch --all
hynek marked this conversation as resolved.
Show resolved Hide resolved
```

---

To avoid committing code that violates our style guide, we strongly advise you to install [*pre-commit*] and its hooks:
Expand Down
1 change: 1 addition & 0 deletions changelog.d/1105.change.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Added instructions for pull request workflow to `CHANGELOG.md`.
hynek marked this conversation as resolved.
Show resolved Hide resolved