carpentries-incubator / docker-introduction

Reproducible Computational Environments using Containers
https://carpentries-incubator.github.io/docker-introduction/
Other
44 stars 48 forks source link

New _extras lesson on sequence alignments #162

Open jsgro opened 2 years ago

jsgro commented 2 years ago

I changed the name of my cloned version from gh-pages to jys-docker-pages which automatically closed my previous pull request. I made more revisions to the file sequence_alignment.md and added sequences and directories to the docker-intro directory, also added the same within the files/docker-intro.zip file. Pairwise alignment exercises were derived from Docker – Beginner Biologist 3. Multiple sequence alignment exercises were derived from SARS-CoV-2-Spike-alignment.

dme26 commented 2 years ago

Thanks for this PR: the exercise looks like it could well suit a key audience in terms of scientific domain. I don't have the domain experience to review the content, though, so suggest that others would be better placed to do so.

A much more minor point: the git commits here look to project a more complex history than I think is the case. If there are subsequent changes that reorganise the commits involved, perhaps unnecessary commits could be squashed?

sstevens2 commented 2 years ago

Hi @jsgro, working on reviewing this episode.
Some early suggestions -

  1. Could you change the name of sequence_alignment.md to e03-sequence-alignment.md to match the format of our other extra episodes?
  2. Could you add a section in the docker-image-examples.md episode that links to the new example?
  3. Instead of adding files to the docker.zip and directly to extras, could you add all the files for this example to the same zipped folder and include that instead?
jsgro commented 2 years ago

Hi Sarah,

Did you all have a coding marathon yesterday on this... that I missed?

I am replying by email because I cannot find this specific conversation on gihtub.

I did reply on GitHub to 2 comments you made about adding a DOI and some italic text. Both are resolved (as not change required.)

For "Squashing" comments myself the only way I was able to do it was by using GitHub-Desktop. But then the only way to "push" back to GitHub is to "force push" that give me this warning:

A force push will rewrite on origin/jys-docker-pages​. Any collaborators working on this branch will need to reset their own local branch to match the history of the remote

In theory I am the only one working on this branch... so it would be OK.

It's been many months that I worked on this, and I find it difficult to get back in and relearn GitHub again, and it is difficult to get back in the train of thoughts to make more modifications.

Just trying to be consciously helpful! and not mess things up.

Thanks for your help!

Jean-Yves

jsgro commented 2 years ago

Hello Sarah and A Turner.

I think that I was able to squash some of the commits (by Github Desktop - I don't have Tower.) However, I see that there are many more, from Oct 5 and Oct 12. But GitHub Desktop is now replying for any of these that I try to merge:

Unable to squash. Squashing replays all commits up to the last one required for the squash. A merge commit cannot exist among those commits.

At this point, since it's been 4 months, I am completely lost as I have not use Git or GitHub since then... Sarah, if you want to "merge-squash" that is fine if Credit is lost.

JYS

P.S. Also for the minor edits from yesterday by Sarah, I don't have write permission feel free to "press the button" to implement them as I cannot...

sstevens2 commented 2 years ago

I’m not sure you saw all of my inline comments, so you might want to look there before you push the squashed changes.

I attended a lesson development co-working session and spent some time reviewing this PR. I’m trying to get some issues settled so we can send this lesson through a formal peer review process.

I think you should be okay to overwrite the branch once you’ve made the changes I suggested inline. I was working only in the GitHub web interface. I’ll re-review your changes (when I have time) once you’ve pushed the squashed branch.

jsgro commented 2 years ago

I have implemented the change suggestions that were added yesterday. For all except a couple of them the suggestion was implemented exactly as suggested and the conversation marked as "resovled."

About the request to change the ZIP file, and the various content name and directories, I am not sure I understand exactly what is requested. Furthermore, this was done 4 months ago, and I am afraid that changing the file and folders name or location will need more editing... And after 4 months I am not sure that I can do that without adding more mistakes.

sstevens2 commented 2 years ago

Hi @jsgro. I'll try to go through the current PR soon but can clarify the zip file suggestion a bit more here. I'm suggesting we decouple the files for this exercise from the files for the main part of the lesson. I think it will be useful since this is an optional example that will only be used when the instructor wants to use a genomics example.

This means that you would remove the changed docker-intro.zip file and other fa and fasta files from this PR. Then instead you put all the fa and fasta files you included in this PR in a new folder docker-alignment./ along with any other files needed for this episode, and zip that up and include it in the files/ folder instead. So when someone teaches this episode, they will have the learners download that folder.

jsgro commented 2 years ago

I have done this as suggested by Sarah: This means that you would remove the changed docker-intro.zip file and other fa and fasta files from this PR. Then instead you put all the fa and fasta files you included in this PR in a new folder docker-alignment/ along with any other files needed for this episode, and zip that up and include it in the files/ folder instead. So when someone teaches this episode, they will have the learners download that folder.

Thanks Sarah, that was very helpful.

I also added the entry in docker-image-examples.md as was requested at some point.