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

[AT-4086] Enable linting for asset modules #564

Merged
merged 10 commits into from
Mar 14, 2024
Merged

Conversation

javidq
Copy link
Contributor

@javidq javidq commented Mar 13, 2024

Enable linting for asset + coverage + fixtures, asset_searcher and http_adapter.

Items:

  • Ran test & live-tests
  • Implemented (new) unit tests
  • Removed credentials
  • Removed argument pointing to staging
  • Updated documentation

For release:

  • Bumped version
  • Added changelog
  • Updated announcement banner

@javidq javidq marked this pull request as ready for review March 13, 2024 15:30
@javidq javidq changed the title Enable linting for asset modules [AT-4086] Enable linting for asset modules Mar 13, 2024
Copy link
Contributor

@seedlit seedlit left a comment

Choose a reason for hiding this comment

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

left a couple of comments

CHANGELOG.md Outdated
@@ -30,6 +30,14 @@ You can check your current version with the following command:

For more information, see [UP42 Python package description](https://pypi.org/project/up42-py/).

## 0.37.0a11

**March 13, 2024**
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we will need to update the date.

@@ -80,27 +79,20 @@ def test_asset_update_metadata(asset_mock):
assert updated_info["tags"] == ["othertag1", "othertag2"]


def test_asset_get_download_url(assets_fixture):
Copy link
Contributor

Choose a reason for hiding this comment

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

why are we removing this test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. It's not a public method
  2. It is used by asset::download (covered by tests) and asset::get_stac_asset_url (not covered anyway)

Copy link

sonarcloud bot commented Mar 14, 2024

@javidq javidq merged commit d3d2e42 into master Mar 14, 2024
3 checks passed
@javidq javidq deleted the AT-4086-asset-linting branch March 14, 2024 12:31
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.

2 participants