sourmash-bio / sourmash

Quickly search, compare, and analyze genomic and metagenomic data sets.
http://sourmash.readthedocs.io/en/latest/
Other
473 stars 80 forks source link

github etiquette and contribution tips for sourmash #2098

Open ctb opened 2 years ago

ctb commented 2 years ago

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 https://github.com/sourmash-bio/sourmash/pull/1808

ctb commented 2 years ago

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
  5. 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 commented 2 years ago

Step 5. Profit!!!

ctb commented 2 years ago

I thought

pytest tests -k storage_convert -r x

was a neat way to debug the test_storage_convert xfail from https://github.com/sourmash-bio/sourmash/pull/1739.

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