Skip to content

refactor: move hathor settings to yaml configuration files [part 2/2]#597

Closed
glevco wants to merge 4 commits intorefactor/settingsfrom
refactor/remove-old-settings
Closed

refactor: move hathor settings to yaml configuration files [part 2/2]#597
glevco wants to merge 4 commits intorefactor/settingsfrom
refactor/remove-old-settings

Conversation

@glevco
Copy link
Copy Markdown
Contributor

@glevco glevco commented May 11, 2023

Depends on #593

Acceptance Criteria

  • Remove unused python configuration files

@glevco glevco requested a review from pedroferreira1 May 11, 2023 16:52
@glevco glevco self-assigned this May 11, 2023
@glevco glevco requested review from jansegre and msbrogli and removed request for jansegre May 11, 2023 16:55
@glevco glevco marked this pull request as ready for review May 11, 2023 16:56
@glevco glevco requested a review from jansegre as a code owner May 11, 2023 16:56
@glevco glevco removed the request for review from jansegre May 11, 2023 16:56
@codecov
Copy link
Copy Markdown

codecov bot commented May 11, 2023

Codecov Report

❗ No coverage uploaded for pull request base (refactor/settings@e0c35ad). Click here to learn what that means.
The diff coverage is n/a.

@@                 Coverage Diff                  @@
##             refactor/settings     #597   +/-   ##
====================================================
  Coverage                     ?   83.63%           
====================================================
  Files                        ?      229           
  Lines                        ?    19667           
  Branches                     ?     2695           
====================================================
  Hits                         ?    16448           
  Misses                       ?     2621           
  Partials                     ?      598           

@glevco glevco force-pushed the refactor/remove-old-settings branch from 0353479 to fec0177 Compare May 11, 2023 22:36
@glevco glevco requested review from jansegre and removed request for pedroferreira1 May 13, 2023 03:59
@glevco glevco force-pushed the refactor/remove-old-settings branch from fec0177 to 02dcf9e Compare May 15, 2023 19:00
@glevco glevco force-pushed the refactor/settings branch from 48715e3 to f1e1ae8 Compare May 15, 2023 22:48
@glevco glevco force-pushed the refactor/remove-old-settings branch from 02dcf9e to a123809 Compare May 15, 2023 22:51
Copy link
Copy Markdown
Member

@jansegre jansegre left a comment

Choose a reason for hiding this comment

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

The changes seem correct. But I would prefer to either hold them off for 0.55.0 (instead of 0.54.0 which is on track to have YAML settings support), or have a better mechanism for steering people to the correct way to use settings if they are using the old mechanism (mentioned here).

@glevco glevco force-pushed the refactor/settings branch 2 times, most recently from 662a06b to 6e0ded5 Compare May 16, 2023 22:16
@glevco glevco force-pushed the refactor/remove-old-settings branch from a123809 to f2a7bee Compare May 16, 2023 22:41
@glevco glevco force-pushed the refactor/settings branch from 6e0ded5 to 69f593a Compare May 17, 2023 15:53
@glevco glevco force-pushed the refactor/remove-old-settings branch from 3ba9fcb to e1a229e Compare May 17, 2023 19:26
@glevco
Copy link
Copy Markdown
Contributor Author

glevco commented May 17, 2023

@jansegre

The changes seem correct. But I would prefer to either hold them off for 0.55.0 (instead of 0.54.0 which is on track to have YAML settings support), or have a better mechanism for steering people to the correct way to use settings if they are using the old mechanism (mentioned here).

Backwards compatibility has been added as explained in #593 (comment). But if people are inheriting from our python modules, I agree we should not delete them in a transitioning release. What do you think should be done with this PR? Should I close it and reopen when it's time, or do we just leave it here, waiting?

@glevco glevco requested a review from jansegre May 17, 2023 20:50
@glevco
Copy link
Copy Markdown
Contributor Author

glevco commented May 17, 2023

I'm closing this PR and moving it to the Parking Lot. We'll release the backwards compatible version in #593 , and eventually reopen this one when we are sure the python modules can be removed.

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.

3 participants