Changing the names of the calculations so that they are consistent.#68
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #68 +/- ##
========================================
Coverage 91.21% 91.21%
========================================
Files 33 33
Lines 2652 2652
========================================
Hits 2419 2419
Misses 233 233
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
@chrisjsewell, @sphuber |
pyproject.toml
Outdated
| "lammps.md.multi" = "aiida_lammps.calculations.lammps.md_multi:MdMultiCalculation" | ||
| "lammps.optimize" = "aiida_lammps.calculations.lammps.optimize:OptimizeCalculation" |
There was a problem hiding this comment.
What about these and the CombinateCalculation?
There was a problem hiding this comment.
Thanks I have missed those ones!
Should I also do the same in the parsers? or make a different PR for that?
There was a problem hiding this comment.
Yeah I would also add the parsers to this PR.
There was a problem hiding this comment.
On a side note: do you have a specific reason for merging pull requests, instead of squash merging? Or are you just using it because it is the default. If you are just using it by default, may I suggest we start using squash merge instead, at least for PRs that don't have neatly crafted commits that should be kept separate? This keeps the history on the main branch more linear and a lot more easy to navigate. These merge commits and intermediate commits from feature branches are almost never useful. We are using this approach in all other aiidateam repos as well and works really well.
There was a problem hiding this comment.
Basically because it is the default.
With squash merging you refer to squashing all the merges in a PR into a single commit right? which in principle could have been many commits in the fork. Or is it something else?
I think that keeping the main branch history clean would be very nice I agree.
Any thoughts on this @chrisjsewell ?
There was a problem hiding this comment.
With squash merging you refer to squashing all the merges in a PR into a single commit right?
Indeed. Next to the Merge pull request button is a little arrow where you can select the merge method. There you select "Squash and merge" instead and then click the button. It will open a text editor to format the final commit message. By default it takes a concatanation of commit messages of all commits. I usually edit this to be a single nice final commit message.
Chris is on a holiday, so not sure if he will see this. But I am pretty sure he also uses squash merge (where applicable). Note that there are still cases in which you don't want to squash, but in that case, the PR author needs to properly prepare the commits in the PR branch as they should be merged into the main branch. Then we use rebase and merge instead of squash. But for most PRs, jsut a single commit is sufficient and so we simply squash.
There was a problem hiding this comment.
Nice! I'll do it then for this PR, I normally did the squashing by hand, but if github can do it for me I'm happy about it.
|
Okay I have changed the Parsers and calculations so that they all have the same standard way. @sphuber you can check again if you want. And I'll do the merging with squashing this time to try to keep the main brain a bit cleaner. I might need to do some of that by hand later to fix some of the superfluous commands introduced by the other PRs. |
I wouldn't worry about that now, can get tricky if you merged in branches and start messing with the history. Let's just use squash-merge and rebase-merge from now on. |
2 participants
Changes the names of the calculations so that the follow the template
Lammps{type}Calculation.Addresses #56