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

github etiquette and contribution tips for sourmash #2098

Open
ctb opened this issue Jul 1, 2022 · 3 comments
Open

github etiquette and contribution tips for sourmash #2098

ctb opened this issue Jul 1, 2022 · 3 comments

Comments

@ctb
Copy link
Contributor

ctb commented Jul 1, 2022

from a slack thread, a few links and tips we could add to docs for new sourmash developers -

@mr-eyes says:

Alternatively, if the edit is minor, you can suggest a change directly on Luiz's PR. Here's a tutorial: https://www.youtube.com/watch?v=D8Drb2OKibs&ab_channel=AndreaMussap

@luizirber sez:

please never commit directly to other people branches -
I use rebase frequently, and sometimes it can create a bit of a mess if I'm unaware something changed while I was working locally. It is not the end of the world, I know how to recover, but I would like to avoid the effort =P
(The main offender is when someone clicks the “update branch” button in someone else's PR, that's why we don't recommend doing it)

I added:

I will sometimes use PRs to suggest changes to other people’s branches, which leaves them with the choice of merging, or just taking inspiration.

more @luizirber -

for way too much info:
https://github.blog/2022-06-30-write-better-commits-build-better-projects/
our "unit" of work in sourmash is a PR. once it is reviewed and approved, it gets turned into one commit that shows up in the latest branch.
Inside the PR, during development? you can do as many commits you want. Like, some people even do 250 of them =P
#1808

@ctb
Copy link
Contributor Author

ctb commented Jul 11, 2022

on PR reviews -

@ccbaumler -

Just clarifying the process for reviewing and merging a pull request. It goes like this:

  1. Look through changes
  2. Click the "Add your review" above the checks, or navigate to files changed​ and click "review changes"
  3. Comment, approve, or request changes (E.g. "LGTM", click approve, and click submit review)
  4. Then click merge pull request within the pullrequest

Is there a write-up or documentation on the checks that run and which checks are selected to run?

I respond:

All but (4)! usually we leave things to the PR author to merge.

There’s no clear writeup on the checks that run, I don’t think. But the important bit is that it runs all the tests against all the versions of Python.

@ccbaumler
Copy link
Contributor

Step 5. Profit!!!

@ctb
Copy link
Contributor Author

ctb commented Aug 22, 2022

I thought

pytest tests -k storage_convert -r x

was a neat way to debug the test_storage_convert xfail from #1739.

ref https://docs.pytest.org/en/7.1.x/how-to/skipping.html

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

No branches or pull requests

2 participants