PRBEM / IRBEM

IRBEM-LIB provides routines to compute magnetic coordinates for any location in the Earth's magnetic field, to perform coordinate conversions, to evaluate geophysics/space-physics models, and to propagate orbits in time.
https://prbem.github.io/IRBEM/
Other
33 stars 14 forks source link

Repository updates for github workflow #1

Closed drsteve closed 3 years ago

drsteve commented 3 years ago

This pull request adds a few things to make life on github a little easier for contributors and maintainers alike.

I've added:

Some suggestions for merging this and future PRs:

AntoineBrunet commented 3 years ago

@mshumko I had not merged the PR after the review, it should be closed now.

drsteve commented 3 years ago

Thanks @AntoineBrunet and @mshumko for the feedback and questions. Updating the README is a good idea, but I was hesitant to do that in this PR. I really just wanted to add the new templates, etc. Since @AntoineBrunet had a question about that, and it's easy to add commits to a PR, I'd assume that's why he didn't merge.

IIRC (I'm on mobile so it's harder to check) the repo has README.install and not a markdown document. It's probably more of a discussion for an issue than for this (now merged) PR, but since it's really install instructions and not a general readme it'd be useful to expand with, e.g., a brief description of library capabilities, notes to point folks to the issues and discussions on the GitHub page if they have problems (should be first port of call rather than direct contact), point folks to files with more info on workflow, etc.

Also, I note that this was done as a merge commit. Is that going to be the preferred method for merging going forward? Or is it a function of still getting familiar with the options, setup, and how the different strategies manifest? Either way, it's good to document and I'd suggest adding the repo's merge strategy to the WORKFLOW.md once it's agreed on (as a reference for developers and as info for contributors.

AntoineBrunet commented 3 years ago

@drsteve Sorry for the mixup, I agree with you that rebasing is better, I did not notice the option when merging the PR, but from now on I suggest we use the rebase strategy.