Skip to content

Fix/1864setsnapshotspath #1867

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 22 commits into from
Sep 13, 2024
Merged

Fix/1864setsnapshotspath #1867

merged 22 commits into from
Sep 13, 2024

Conversation

buhtz
Copy link
Member

@buhtz buhtz commented Sep 8, 2024 •
edited
Loading

Complex refactoring and code movement. ⛑️ Behavior of BIT shouldn't be modified. 😨

This PR fix refactoring issue #1864 and prepares for a new config management class (PR #1850) and refactoring SettingsDialog code (PR #1865). The problem this PR solves was that the method Config.setSnapshotsPaths() has to much logic in it not belonging in that method nor into that class. I moved and "parked" the code in the tools.py module.

The current solution give me the ability to easier decouple the code in SettingsDialog.saveProfile() in context of PR #1865.

Of course I modified several other things but tried to keep that PR as simple as possible to reduce the risk of introducing bugs.

Fix #1864

@buhtz buhtz self-assigned this Sep 8, 2024
@buhtz buhtz added PR: Waiting for review PR won't be merged until review and approval from a member of the maintenance team. Code Quality About code quality, refactoring, (unit) testing, linting, ... labels Sep 8, 2024
daviewales
daviewales reviewed Sep 9, 2024
View reviewed changes
@buhtz
Copy link
Member Author

buhtz commented Sep 9, 2024 •
edited
Loading

Slightly modified to totally decouple the Config class from the parked code. Of course that "solution" is not permanent.

daviewales
daviewales reviewed Sep 10, 2024
View reviewed changes
@buhtz buhtz added PR: Merge after creative-break Merge after creative-break (min. 1 week) and removed PR: Waiting for review PR won't be merged until review and approval from a member of the maintenance team. labels Sep 10, 2024
@buhtz
Copy link
Member Author

buhtz commented Sep 10, 2024

Solved merge conflicts caused by merging PR #1860 (Warn about NTFS).

@buhtz buhtz merged commit 67e93bd into bit-team:dev Sep 13, 2024
1 check passed
@buhtz buhtz deleted the fix/1864setsnapshotspath branch September 13, 2024 07:19
This was referenced Oct 1, 2024
Closed
Merged
buhtz added a commit that referenced this pull request Oct 3, 2024
@buhtz
fix: Explicit "mode" argument for Config.get_snapshots_mountpoint() (#…
b811bd5 
…1892)

Method Config.get_snapshots_mountpoint() (introduced in PR #1867) now use mode as argument. Fix #1892.

Additionally replaced "pycodestyle" with "flake8" linter. And check the version of the linters because their rules are quit different between versions.
@buhtz buhtz removed the PR: Merge after creative-break Merge after creative-break (min. 1 week) label Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@daviewales daviewales daviewales left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@buhtz buhtz

Labels

Code Quality About code quality, refactoring, (unit) testing, linting, ...

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Blocking/Refactor: Make Config.setSnapshotsPath() dumb

2 participants

@buhtz @daviewales