snakemake / snakemake-executor-plugin-googlebatch

Snakemake executor plugin for Google Batch
MIT License
3 stars 5 forks source link

test: Test cos #46

Closed cademirch closed 7 months ago

cademirch commented 7 months ago

Adds "-e PYTHONUNBUFFERED=1" to snakemake container options to flush snakemake stdout/stderr so they show up in google logs

cademirch commented 7 months ago

Oops I am trying to pull this into main. I can change that...

vsoch commented 7 months ago

@cademirch this is the correct branch to PR to - either you can keep your branch (but use commits from the other so they are properly attributed) or rebase in this context. What I discourage is to take someone else's code and commit/present it as your own.

vsoch commented 7 months ago

And if you intended to rebase against main, I don't think you did that because changes from the testing-cos branch were present!

cademirch commented 7 months ago

@vsoch Agh, sorry about that. Definitely did not intend and don't want to take credit for other's commits! For my sake, how should have I done this? My quick google led me to do this:

git checkout -b test--cos upstream/testing-cos
git rebase main
... add my changes, push to my fork, open PR
vsoch commented 7 months ago

My guess is that your main branch has commits not present here, meaning at some point you did a PR against snakemake from your main branch that was squashed (and now you have extra commits).

What you want to do to fix this is to clone snakemake fresh, force push the main branch to your main, and then moving forward never do a PR from your main to snakemake main. Always work from feature branches.

vsoch commented 7 months ago

Also if you are new to rebasing, I made a very dumb video a few years ago, haha. https://youtu.be/9F4RE2_yn6I

But rebasing only works if your main is in sync with the upstream's main!

cademirch commented 7 months ago

Awesome. Thank you for explaining this and bearing with my lack of git skills. Weirdly my main branch is not ahead of this one - so I'm not even sure what I did, but some how messed up anyways 😅. If it's okay, I'll close this and make a new draft. Quick question - do I even need to rebase against main? Or will you able to rebase when you merge testing-cos into main?

vsoch commented 7 months ago

Quick question - do I even need to rebase against main? Or will you able to rebase when you merge testing-cos into main?

If you checkout your new feature branch directly from main you should not need to! Rebasing is only when you have commits you want to fixup or squash relative to some other branch, or you pulled an update upstream main to your main, and want to rebase to ensure the changes there are in your feature branch.

cademirch commented 7 months ago

I guess I'm confused since if I checkout from main I wouldn't have the testing-cos code since its not in main?

Sorry for all the questions. I've created a draft #47, which just adds my two changes on top of the testing-cos branch, which I think is appropriate.

vsoch commented 7 months ago

I guess I'm confused since if I checkout from main I wouldn't have the testing-cos code since its not in main?

Every new feature branch is generally checked out from main, but you are right if you are doing a PR to testing-cos it's ok to checkout from there too.