search

diffpy / diffpy.nmf_mapping

The code for the nmfMapping App on difpy-cmi
Other
3 stars 3 forks source link

Cookierelease: Folder setup 1/3 (Excluding src and doc) #7

Closed bobleesj closed 3 months ago

bobleesj commented 3 months ago

Please review. @sbillinge src/ and doc/ will be handled in the next PRs.

sbillinge commented 3 months ago

Looks good. Just some issues with copyright dates maybe?

bobleesj commented 3 months ago

Looks good. Just some issues with copyright dates maybe?

https://github.com/diffpy/diffpy.nmf_mapping/commit/d40b80700d907648b2622cfa240bda7bfb8d0182

I see the first commit being 2022. I will make a change.

bobleesj commented 3 months ago

@sbillinge I also have a general question on a GH workflow.

Assume I made a mistake in a commit, and I want to revert my commit (to fix the problem). I also do not want the reviewer to waste time reading the mistake.

I see some options such as 1) force push 2) git revert. I see that git revert indicates a clear undoing of the previous changes. I feel uncomfortable with the word "force".

What is the best practice?

bobleesj commented 3 months ago

Please review:

Citations added to README.rst, URLs working.

Screenshot 2024-08-11 at 11 17 50 AM
sbillinge commented 3 months ago

Please review:

Citations added to README.rst, URLs working.

Screenshot 2024-08-11 at 11 17 50 AM

Nice job. For the second paper add the words "and please consider citing " before it.

bobleesj commented 3 months ago

Nice job. For the second paper add the words "and please consider citing " before it.

Yup. Added. I will reflect those in doc as well in the following PR.

Screenshot 2024-08-11 at 4 07 35 PM
sbillinge commented 3 months ago

@sbillinge I also have a general question on a GH workflow.

Assume I made a mistake in a commit, and I want to revert my commit (to fix the problem). I also do not want the reviewer to waste time reading the mistake.

I see some options such as 1) force push 2) git revert. I see that git revert indicates a clear undoing of the previous changes. I feel uncomfortable with the word "force".

What is the best practice?

never force push. I don't want to merge any force-pushes. Some repo's have this set as a rule. just revert if it is a small commit. The revert applies the anti-change, so the change is done then undone, which is ok if it is small, but if it is big it is generally better to kill the branch and do a new PR. Again, another reason to keep the edits on each PR to be small.

bobleesj commented 3 months ago

never force push. I don't want to merge any force-pushes. Some repo's have this set as a rule. just revert if it is a small commit. The revert applies the anti-change, so the change is done then undone, which is ok if it is small, but if it is big it is generally better to kill the branch and do a new PR. Again, another reason to keep the edits on each PR to be small.

Thank you. Well noted. @sbillinge