CDCgov / seqsender

Automated Pipeline to Generate FTP Files and Manage Submission of Sequence Data to Public Repositories
https://cdcgov.github.io/seqsender/
Apache License 2.0
32 stars 10 forks source link

Update readme with doc style info #6

Closed leebrian closed 6 months ago

leebrian commented 1 year ago

To help users and potential collaborators, please update the readme to follow the flu doc style and explicitly call out how you like folks to submit issues, test issues, commit (eg, dev branches vs main vs releases).

After your changes a user or collaborator will be able to understand how you work on and make changes to seqsender and how to expect to watch for in progress work, completed work and new releases.

dthoward96 commented 1 year ago

See https://github.com/CDCgov/seqsender/issues/4#issuecomment-1578580661 for update to this issue

leebrian commented 1 year ago

thanks Dakota, any estimate on this? I want to finalize our SOP and submit it for approval and want to use the results from you applying it as a test that the practices are decent for scientists to use.

dthoward96 commented 1 year ago

Definitely by the end of the week. I'm bouncing around on a couple different projects this week but I'm working on testing the bug fixes and then I'll have the doc's updated.


From: Brian Lee @.> Sent: Friday, June 23, 2023 10:54 AM To: CDCgov/seqsender @.> Cc: Howard, Dakota (CDC/DDID/NCIRD/ID) (CTR) @.>; Assign @.> Subject: Re: [CDCgov/seqsender] Update readme with doc style info (Issue #6)

thanks Dakota, any estimate on this? I want to finalize our SOP and submit it for approval and want to use the results from you applying it as a test that the practices are decent for scientists to use.

— Reply to this email directly, view it on GitHubhttps://github.com/CDCgov/seqsender/issues/6#issuecomment-1604398137, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AOCAVN47BSPCVC7OKYZ3NK3XMWUZDANCNFSM6AAAAAAYPLEHCM. You are receiving this because you were assigned.Message ID: @.***>

leebrian commented 1 year ago

Would you please post an estimate for when you'll have this change ready for review? It wasn't a high priority initially, but now a seemingly simple change has been open since May 25, so it seems to be part of an overall problem with other open issues that are unclear what the progress is around them.

leebrian commented 1 year ago

@dthoward96 what's the new ETA? this is really taking forever. I estimate it should take an hour to update the readme.md with whatever's required for the SOP but that may be way off. Please work on this as it will help me finalize the SOP with any changes we need to make before releasing it to out to the rest of the group.

dthoward96 commented 1 year ago

There has been a lot of changes that have taken place that requires a lot more documentation. The submission process changes depending on the organism as not all options are available, GISAID is no longer packaged with SeqSender and must be installed by the user, and Table2asn integration also heavily changes the Genbank submission process. I've also been working on making short GIF's to provide visual examples to cover different sections and answer general questions that seem to regularly pop up. I've setup a meeting next week with Ben and Reina to go over any issues with MIRA/Docker version so I can finalize the documentation for SeqSender around the middle/later half of next week.

leebrian commented 1 year ago

Thanks. I expect to see it next week and will keep an eye out.

Finalizing is a good goal if possible, Burt just start with any progress. Drafts are good and you should have frequent commits showing incremental progress.

leebrian commented 1 year ago

Thanks. I expect to see it next week and will keep an eye out.

Finalizing is a good goal if possible, Burt just start with any progress. Drafts are good and you should have frequent commits showing incremental progress.

leebrian commented 12 months ago

@dthoward96 What's the new ETA since the August 31st estimate didn't come true?

dthoward96 commented 12 months ago

@dthoward96 What's the new ETA since the August 31st estimate didn't come true?

From the meeting I had with @nbx0 and @rchau88 on the 5th, Ben wanted Reina and I to combine our versions into one. Reina had some work she needed to finish and she encountered a bug for GISAID influenza submissions which we both met on Monday to go over. I just pushed my updates for the bug she encountered, so once she has had time to do a production submission for that section and implement the changes she was working on we're then going to meet and combine our code and write out the documentation. Once she's back next week we should be able to come up with a new ETA.

rchau88 commented 11 months ago

Hi @leebrian, @dthoward96, and @nbx0,

I'm back and will start working on incorporating Dakota's newest fixes to GISAID FLU submissions. I would like to run all test cases on FLU and SARS-COV-2 submissions in order to make sure the tool is functioning as expected before Dakota and I merge it into production. I had written extensive documentation about the newly reconstructed version of seqsender. Once we know the pipeline is thoroughly tested, it will be ready to publish on Github.io. I expected the new ETA to be by next Wednesday 9/27. The reason is I need this week to implement the fixes, run testing, troubleshoot, and rewrite some documentation as there will be new changes after testing. Hope this works with everyone!

leebrian commented 11 months ago

Thanks for the update Reina. This sounds good and I think goes beyond the minimum and will make seqsender much more easily understood by users.

We also need to think about how to make smaller, incremental changes rather than big, giant changes. As you update and bring everything into the repo, please propose what you think will help developers (really just you and Dakota, but hopefully others will be contribute) make smaller commits. Smaller, more frequent commits are easier to test, help inform users of what's happening, and get us closer to others being able to contribute code, tests, docs, etc.

rchau88 commented 11 months ago

I agreed with Brian. Once the tool is stable and has been thoroughly tested. I hope with documentation, it will allow collaborators and users to make incremental changes and apply fixes later on.

dthoward96 commented 6 months ago

@leebrian The master branch is up to date and covers everything under the SOP. I attached links to each relevant section where applicable. I also bolded my questions and suggestions for possible recommendations/requirements. Let me know if there is any changes you'd like me to make to it.

5.3.1 Adhere to all Required Practices in CDC GitHub Practices for Open Source Projects and consider all Recommended Practices in CDC GitHub Practices for Open Source Projects as part of software design.

The last section for security scanning and review is kind of confusing it might be better to have this more detailed. There are a lot more security and scanning options and it might be helpful to list which should be enabled. Below is a couple of examples: - Security Policy is a default template for how users can report security issues that's built into github. - Secret Scanning scans for credentials and other info that shouldn't be uploaded to public githubs and reports it to the repo maintainers and can block pushes when it detects them. - Vulnerability Scanning performs automated code vulnerability scanning and reports it to maintainers.

This section appears in the documentation style, however, the documentation style says its only recommended and not required. Also, it says to put a "#" in front of each keyword. Is that the correct format or without?

5.3.3 Update your readme.md to follow the Documentation Style.

For clarification on the authors section. Rmarkdown automatically builds its citations and authors list based on certain keywords which is why it labels users as "Authors". There is only a small list of options to pick from and these seemed the most correct based on the options and is how MIRA also labels their. For the attributions, license, and limitations section I added a "code attributions" section since we didn't have external code attributions. The other sections are already covered under the required sections for the CDC required GitHub practices, so it seems a little redundant to include it over again so I left those as they were.

It might be helpful to request/require some custom/developer provided github workflows. - One example might be having alot of the required sections that are fairly consistent across all github repos stored in one central documentation section. We could then provide/require a github workflow that on a interval pulls from that documentation repo and updates any files and can append any corrected information to your README. That way we could have FLU controlled language for usage, license, disclaimers that are automatically updated when the documentation repo is updated. - Some other recommended workflows could be the ones provided by Docker. For example there are some already provided workflows that can automatically generate the newest DockerFile whenever there is a push to master and it can even upload that new version then automatically to dockerhub and other repos.

Another suggestion I have is also getting users to store their containers in GitHub Packages. Github now has a build in repo that can hold Docker/Singularity containers and other types.

leebrian commented 6 months ago

Thanks, this looks good.

I removed the # from the documentation style page keywords.

I updated 5.3.1 to explicitly call out "For security scanning and review, emable at least Dependency Graph, Dependabot alerts, Dependabot security updates, Dependabot version updates, Code scanning, Secret scanning, and push protection."

Good suggestions on having a centralized repo. Currently, that's sort of the template repo. All that boilerplate rarely changes so it may not be worth the effort to set up a job.

Good point on the github packages. I'm not sure why we use dockerhub instead of github packages. I think it was just Reina's preferences. I know some groups use packages so it is worth exploring.

I think this one is done, so closing it out. Thanks!