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

[Breaking Change]: Remove save file func #159

Merged
merged 10 commits into from
May 25, 2022
Merged

Conversation

Milind220
Copy link
Collaborator

This PR will address the breaking change of removing the save to file functionality that Ozone currently offers.

@Milind220 Milind220 added this to the version 3.0 milestone May 23, 2022
@Milind220 Milind220 self-assigned this May 23, 2022
@Milind220 Milind220 marked this pull request as draft May 23, 2022 02:31
@Milind220
Copy link
Collaborator Author

Milind220 commented May 23, 2022

Work that needs to be done:

  • Modify tests to work with lack of save file feature.
  • Re recording cassettes to make tests work

@Milind220 Milind220 linked an issue May 23, 2022 that may be closed by this pull request
@Milind220
Copy link
Collaborator Author

@lahdjirayhan I believe this PR is complete :) I'll check again tomorrow

Copy link
Contributor

@lahdjirayhan lahdjirayhan left a comment

Choose a reason for hiding this comment

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

The presence of * will force users to give city or city_id by name. Giving arguments by position will raise an error.
See

In my opinion, this avoids mixing up whether the user is giving a city or city_id.

But I think getting rid of it is also fine since the arguments are checked immediately.

@lahdjirayhan
Copy link
Contributor

@Milind220

Looks good to me!

Could you also please update the README because I think there's an example that still shows data_format argument being used.

@Milind220 Milind220 marked this pull request as ready for review May 23, 2022 16:24
@Milind220
Copy link
Collaborator Author

Milind220 commented May 25, 2022

@lahdjirayhan Hey, any idea why the linter checks might be failing?

EDIT: nvm I think this next commit should fix that. flake8 lint was flagging an unused import (I think my pre-commit hooks weren't working - reinstalled them now)

@Milind220
Copy link
Collaborator Author

Milind220 commented May 25, 2022

Okay I'm trying to solve some merge conflicts lol - excuse me if this gets a little messy.

EDIT: Hopefully this fixed it hahaha

@Milind220 Milind220 merged commit c2d5867 into dev May 25, 2022
@Milind220 Milind220 deleted the remove-save-file-func branch May 29, 2022 03:41
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.

[Breaking change]: Remove save data to file functionality
2 participants