Updating the README.md with environment hints for new users.

vdblab / vdblab-shotgun

Shotgun metagenomic sequencing processing pipeline

MIT License

1 stars 1 forks source link

Updating the README.md with environment hints for new users. #74

Closed miraep8 closed 10 months ago

miraep8 commented 10 months ago

Hey all, I added a few of the issues I ran into getting the pipeline working to the README.md to (hopefully) smooth the process for new users in the future!

Happy to continue to make changes to this before merging, let me know what you think and/or would like me to change and/or add!

Cheers!

miraep8 commented 10 months ago

I suspect that the issue with the Full Integration Test failure is because the stable snakemake image it is trying to use to run the tests is using a version of snakemake (ie > 7.31.1) that breaks our code. I can update that to point to a more appropriate image. Will take a closer look into why the linter is failing tomorrow! (and can hopefully resolve both simultaneously)

miraep8 commented 10 months ago

Aha, so good news, setting the smakemake docker image back to version 7.31.1 fixes the integration issues :). Now on to the linter! I suspect this might have something to do with how I am doing my PR.

miraep8 commented 10 months ago

@funnell and @nickp60 this PR is now ready for whenever you get a chance to review the changes. In addition to the changes to the README.md I also had to update the two GitHub actions that run on the pull request in order to get this to work. These changes were:

change the snakemake image used to test the pipeline. I reverted this to v7.31.1 to match the latest version of snakemake that is consistent with the pipeline.
add the full branch history to the checkout command. This is needed for the superlinter to be able to recognize the branch and work properly. I think this likely has to do with how I made the PR (ie forking the repo and then making a PR from a new branch in my forked repo). It seems like this is not how you guys make PRs and I am happy to change/use the way that you do it! As I don't (currently) have write privileges to this repo this was the best (and only) was I was aware of to make a PR.

As always happy to make any changes you request! Cheers!

funnell commented 10 months ago

Thank you so much for doing this @miraep8 this is very helpful 👍

nickp60 commented 10 months ago

This is great! What does the fetch-depth setting do for us here in the action yaml?

miraep8 commented 10 months ago

From my understanding of it the fetch depth = 0 command tells the checkout action to get the commits for all branches and tags. For whatever reason, without this argument the linter runs into a "branch not found" error, I suppose because it has not been supplied with the information related to my PR from the git checkout action. If you check out the documentation for the super linter it actually recommends using this argument for the git checkout action (see below). As to why this was an issue for this PR and not PRs you have made in the past.... I am guessing that it was because I forked the repo and made the PR from there? I noticed I am the only one to have forked the repo so I am guessing that you guys must make your PRs in a more direct way such that the checkout action default was able to grab sufficient information for the super linter to run? Screenshot 2024-01-17 at 5 30 29 PM

miraep8 commented 10 months ago

(more specifically if you are curious, this is the issue that the linter raises without the fetch-depth= 0 switch): Screenshot 2024-01-17 at 5 34 24 PM

nickp60 commented 10 months ago

Cool cool; thanks for troubleshooting that!