search

pieper / SlicerMorphoDepot

Experiments and implementations related to the SlicerMorph MorphoDepot project
Apache License 2.0
0 stars 0 forks source link

Workflow issues to consider #6

Open muratmaga opened 2 weeks ago

muratmaga commented 2 weeks ago

A few things:

  1. Save checkpoint seems like a local commit. Is it really pushing it to the user's fork? If not, this may result in a data loss (if the local repo accidentally deleted or moved, or corrupted).
  2. What happens if the user accidentally clicks on Save and Review Request (or more likely they noticed something and want to update what they asked to be review). Then can go to the issue page and cancel the PR, but I think we need a button to do that in the MD as well. Because I pushed it again (when was testing JS2) and it took me to a page with my existing PR.
pieper commented 2 weeks ago

Yes, once we have the basics working we need to sort out some of the other cases. I want to keep it simple by sticking to a set flow. Regarding your comments:

  1. Here saving a checkpoint is a local commit plus a push to the segmenter's forked repo. Next time they open the issue it opens their local repo if it exists, and pulls from the upstream in case the PI or other segmenters have updated the main repo with material they need to see (like other peoples's segmentations). This means that if they start on a new machine they will get the latest of their previous work and other peoples. Because the seg.nrrd files are named for the issues there should be no overlap if everyone uses the module for their edits.

  2. We can put in more status into in the MD module interface, like pending PRs that you can click and open or delete. If you click save after the PR is already open then it will update the fork with the latest so the PR will also have that. I think this is a good feature but probably needs to be made clear to people somehow.

In general we should also think if we want to avoid wording like "request review" in the UI and instead use github terminology of "open pull request". It would probably make it easier to understand as people get deeper into the real github and/or we think they should use the github interface for some tasks, like commenting on PRs.

muratmaga commented 2 weeks ago
  1. This means that if they start on a new machine they will get the latest of their previous work and other peoples. Because the seg.nrrd files are named for the issues there should be no overlap if everyone uses the module for their edits.

This is great. That's what I wanted to make sure

"request review" in the UI and instead use github terminology of "open pull request".

I agree with this, but there is a learning curve as most people will not be familiar with git and github terms. Perhaps use them both? Button my say "Open Pull Request" (the proper terminology). And the popup for description says something like "Use this button to request a review of your work by the repository owner "

muratmaga commented 2 weeks ago

Yes, once we have the basics working we need to sort out some of the other cases. I want to keep it simple by sticking to a set flow. Regarding your comments:

1. Here saving a checkpoint is a local commit plus a push to the segmenter's forked repo.  Next time they open the issue it opens their local repo if it exists, and pulls from the upstream in case the PI or other segmenters have updated the main repo with material they need to see (like other peoples's segmentations).  This means that if they start on a new machine they will get the latest of their previous work and other peoples.  Because the seg.nrrd files are named for the issues there should be no overlap if everyone uses the module for their edits.

2. We can put in more status into in the MD module interface, like pending PRs that you can click and open or delete.  If you click save after the PR is already open then it will update the fork with the latest so the PR will also have that.  I think this is a good feature but probably needs to be made clear to people somehow.

In general we should also think if we want to avoid wording like "request review" in the UI and instead use github terminology of "open pull request". It would probably make it easier to understand as people get deeper into the real github and/or we think they should use the github interface for some tasks, like commenting on PRs.

So in this context Save Checkpoint would be commit and push? Also it would be good if we can have a short text field to enter some commit text that serve as notes...

pieper commented 2 weeks ago

Right now the github terminology is in tooltips for the buttons, but we could reverse this (hover to see the text).

About notes, yes, I thought about that too. In fact I think we should make it a required field to have a subject line for their commit and then optional to include more description.