search

githubdoe / DFTFringe

DFTFringe Telescope Mirror interferometry analysis Program.
GNU General Public License v3.0
168 stars 59 forks source link

Annulus #125

Closed atsju closed 9 months ago

atsju commented 9 months ago

This PR is rebased to fix the CRLF problems identified here https://github.com/githubdoe/DFTFringe/pull/123#discussion_r1501759809

History and authors are maintained

link #30 #119

gr5 commented 9 months ago

I merged PR 124. Are you going to pull it into this branch? I have to say I really don't understand rebasing and how you fixed the CRLF issue.

Are you going to wait for Dale to make changes and pull those into this branch?

atsju commented 9 months ago

I merged PR 124. Are you going to pull it into this branch?

I integrated #124 here

I have to say I really don't understand rebasing and how you fixed the CRLF issue.

This one was not so easy but basically I rebased the branch and modified the "bad" commit. Fixing the CRLF was explained in #46 I thanked myself to document the PR a lot as I just had to read the article again. This wasn't your fault and should not happen again, it's just that the first commit was older than the fix thus reintroducing previously fixed problems.

Are you going to wait for Dale to make changes and pull those into this branch?

It would be great to fix at least the big issues before merging

gr5 commented 9 months ago

From previous patterns, it appears that Dale only works on code late at night, when Julien is waking up. :)

I'll email him directly.

atsju commented 9 months ago

I did the small fixes. Lets check the build and review one last time to verify there are no more warning. Then we are good to go. Sorry for the little mess in this commit, my editor removes trailing whitespaces automatically and I did not have time to clean this up.

gr5 commented 9 months ago

Merged. I didn't test this time. I always test releases so I want to jump to that step and then test: 1) update RevisionHistory.html 2) Creeate a PR for it.
3) Merge.
4) Create and push new tag.
5) Test installer and latest DFTF before approving release.

gr5 commented 9 months ago

@atsju do I update major or minor version? Basically the only change is to "add" annular processing. It's a big change but for most people who don't have a hole in their mirror, nothing changes.

atsju commented 9 months ago

I would do minor 7.2.0. https://semver.org/ => the change is back compatible