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

added CF override for the metadata bug in ATL19 in crs_txt #7

Merged
merged 3 commits into from
Apr 3, 2024

Conversation

sudha-murthy
Copy link
Collaborator

Description

A short description of the changes in this PR.

Jira Issue ID

DAS-2106

Local Test Steps

PR Acceptance Checklist

  • [X ] Jira ticket acceptance criteria met.
  • CHANGELOG.md updated to include high level summary of PR changes.
  • VERSION updated if publishing a release.
  • Tests added/updated and passing.
  • Documentation updated (if needed).

Copy link
Member

@owenlittlejohns owenlittlejohns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! I only had some minor comments about the release notes. Nothing is a show-stopper.

Because there are a couple of commits in this PR, it'd be good if you could do a couple of things when you merge:

  • Use the "Squash and Merge" option you can get from the dropdown menu of the Merge button.
  • Reformat the commit message at that point so it adheres to our contribution guidelines. It should end up looking something like: "DAS-2106 - Add CF Override for ATL19 crs_txt." (The important bit is prepending the commit message with the ticket number)

CHANGELOG.md Outdated
@@ -1,3 +1,11 @@

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of nits with this description:

  • The issue is sort of a leading backslash, but really it's a leading speech mark that is escaped by a backslash.
  • There's a stray blank line at the top of the file (that shouldn't affect the CI/CD trying to create the release notes, but I'd get rid of it just to be safe).
  • This causes the errors, maybe instead: This causes errors.
  • There's a missing full stop at the end of the paragraph.

I don't think any of these are showstoppers. Just me being a bit pedantic. (I tested the bin/extract-release-notes.sh script with a blank line at the top of CHANGELOG.md and it worked, but had the blank line that might look a bit odd)

{
"Name": "crs_wkt",
"Value": "PROJCS[\"NSIDC Sea Ice Polar Stereographic North\",GEOGCS[\"Unspecified datum based upon the Hughes 1980 ellipsoid\",DATUM[\"Not_specified_based_on_Hughes_1980_ellipsoid\",SPHEROID[\"Hughes 1980\",6378273,298.279411123061,AUTHORITY[\"EPSG\",\"7058\"]],AUTHORITY[\"EPSG\",\"6054\"]],PRIMEM[\"Greenwich\",0,AUTHORITY[\"EPSG\",\"8901\"]],UNIT[\"degree\",0.0174532925199433,AUTHORITY[\"EPSG\",\"9122\"]],AUTHORITY[\"EPSG\",\"4054\"]],PROJECTION[\"Polar_Stereographic\"],PARAMETER[\"latitude_of_origin\",70],PARAMETER[\"central_meridian\",-45],PARAMETER[\"scale_factor\",1],PARAMETER[\"false_easting\",0],PARAMETER[\"false_northing\",0],UNIT[\"metre\",1,AUTHORITY[\"EPSG\",\"9001\"]],AXIS[\"X\",EAST],AXIS[\"Y\",NORTH],AUTHORITY[\"EPSG\",\"3411\"]]"
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. This looks a bit clunky, because the value of the metadata attribute it looooong, but I think this is a good fix. When the collection is fixed up, we'll be able to remove this anyway and clean up the file.

@sudha-murthy sudha-murthy merged commit ce99ba1 into main Apr 3, 2024
3 checks passed
@sudha-murthy sudha-murthy deleted the DAS-2106 branch April 3, 2024 14:47
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.

None yet

3 participants