Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates concerning the Polar Stereographic Grid Mapping #469

Merged

Conversation

sadielbartholomew
Copy link
Member

@sadielbartholomew sadielbartholomew commented Nov 1, 2023

See issue #445 for discussion of these changes.

Release checklist

  • Authors updated in cf-conventions.adoc?
  • Next version in cf-conventions.adoc up to date? Versioning inspired by SemVer.
  • history.adoc up to date?
  • Conformance document up-to-date?

For maintainers

After the merge remember to delete the source branch.
Tags are set at the conclusion of the annual meeting; until then, main always is a draft for the next version.

@sadielbartholomew sadielbartholomew self-assigned this Nov 1, 2023
@sadielbartholomew
Copy link
Member Author

I will address the remaining three checklist points shortly, in a few new commits. For now, FYI the commits so far are the actual changes which implement, with corresponding numbering, the points from my accepted proposal summary #445 (comment).

@larsbarring larsbarring added this to the 1.11 milestone Nov 1, 2023
@davidhassell davidhassell linked an issue Nov 2, 2023 that may be closed by this pull request
@sadielbartholomew
Copy link
Member Author

Quick question: does cf-conventions.adocneed amending given this Issue? I am not sure if my proposal in #445 and this PR to implement it merit my name being added to the 'Additional Authors' list for the overall canonical document. I suspect not, but can someone confirm what if anything needs to be done in this case?

@sadielbartholomew
Copy link
Member Author

(My question of the previous comment has been answered by Jonathan over on the linked Issue. And a merge conflict has resolved itself with my updates.)

Since all updates have been made as discussed on the Issue and I've completed all (applicable) release checklist items, I believe this is now ready for review 🙂

@larsbarring
Copy link
Contributor

I just saw that your PR is marked by github as "This branch has no conflicts with the base branch". Nevertheless the two entries in Table F1 sweep_angle_axis and fixed_angle_axis are shown with the backticks visible. This is becasue the right backticks are preceded by a space. This is something that I (again) saw yesterday in the published document. This time I decided to fix it immediately and made a PR that just removed the two rogue spaces. And then I ventured to immediately merge it myself, because it was such trivial change, and saw that it now looks all right in the published version. I guess that you did not did fetch and rebased on the new main (since yesterday evening), which is very reasonable. But what puzzles me is that this conflict is not identified by github.

@sadielbartholomew
Copy link
Member Author

That's interesting, good spot @larsbarring. gitalmost always handles things like this well under-the-hood, and reports accurately, so I am surprised if it is not behaving right. GitHub itself is more flaky, but the GitHub Status site is not telling of any issues right now.

Is there anything for me to do to remedy the possible merge conflict?

@larsbarring
Copy link
Contributor

I do not know enough about git and github to have an idea as to why this happened. But if you rebase what happens then? Or maybe you could just remove the two rouge spaces from your PR? --- Just a thought...

@sadielbartholomew
Copy link
Member Author

Or maybe you could just remove the two rouge spaces from your PR? --- Just a thought...

If that's essentially what needs to be done (and maybe because it is pure whitespace git and/or GitHub aren't raising this) then it does seem simplest to add a new commit that does that to make the problem go away. Incoming...

@sadielbartholomew
Copy link
Member Author

@larsbarring (after a bit of delay from being distracted by emails) this should now be done in my new commit. Let me know if it looks right to you. And then we should finally be ready for review!

@davidhassell davidhassell merged commit 314264c into cf-convention:main Nov 24, 2023
4 checks passed
@sadielbartholomew sadielbartholomew deleted the i445-polar-stereo-param branch November 24, 2023 14:16
@davidhassell davidhassell mentioned this pull request Nov 27, 2023
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Appendix F: inconsistent name for Polar stereographic map parameter
3 participants