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

Incorporate stac-check linting tool #254

Merged
merged 4 commits into from
Mar 3, 2022
Merged

Conversation

jonhealy1
Copy link
Contributor

@jonhealy1 jonhealy1 commented Mar 2, 2022

Before you submit a pull request, please fill in the following:

Related Issue(s):
#230

Description:

PR checklist:

  • Code is formatted (run scripts/format).
  • Code lints properly (run scripts/lint).
  • Tests pass (run scripts/test).
  • Documentation has been updated to reflect changes, if applicable.
  • Changes are added to the CHANGELOG.

@gadomski gadomski linked an issue Mar 2, 2022 that may be closed by this pull request
.gitignore Outdated Show resolved Hide resolved
src/stactools/cli/commands/lint.py Outdated Show resolved Hide resolved
src/stactools/cli/commands/lint.py Show resolved Hide resolved
@gadomski gadomski mentioned this pull request Mar 2, 2022
@gadomski gadomski added this to the v0.3.0 milestone Mar 2, 2022
@jonhealy1 jonhealy1 marked this pull request as ready for review March 3, 2022 08:10
@jonhealy1 jonhealy1 requested a review from gadomski March 3, 2022 08:14
Copy link
Member

@gadomski gadomski left a comment

Choose a reason for hiding this comment

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

This looks great, only issue now is that the the test with "data-files/linting/20201211_223832_cs2.json" fails. Looks like that item is missing a self link:

$ stac lint tests/data-files/linting/20201211_223832_cs2.json
WARNING A link to 'self' in links is strongly recommended

@gadomski gadomski added the enhancement New feature or request label Mar 3, 2022
Copy link
Member

@gadomski gadomski left a comment

Choose a reason for hiding this comment

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

Ah thanks, I might have had an old version of stac-check installed, so that was probably my mistake.

I'm do think I'm going to open an issue on stac-check re: self links -- though it says they're strongly recommended in the spec documents themselves, the best practices have many cases where they say a self link should only be used in one spot (for relative published catalogs) or never (for self-contained catalogs). I think it's an issue w/ the spec too.

jonhealy1 and others added 4 commits March 3, 2022 05:33
@jonhealy1
Copy link
Contributor Author

Ah thanks, I might have had an old version of stac-check installed, so that was probably my mistake.

I'm do think I'm going to open an issue on stac-check re: self links -- though it says they're strongly recommended in the spec documents themselves, the best practices have many cases where they say a self link should only be used in one spot (for relative published catalogs) or never (for self-contained catalogs). I think it's an issue w/ the spec too.

Ok awesome for sure.

@gadomski gadomski merged commit 51b42d0 into stac-utils:main Mar 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorporate stac-check
2 participants