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

Config: Fix config version downgrade #1770

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

gbjk
Copy link
Collaborator

@gbjk gbjk commented Jan 9, 2025

  • Config: Fix config version downgrade
  • Config: Rename EncryptConfigFile to EncryptConfigData
  • Config: Rename DecryptConfigFile to DecryptConfigData

Fixes #1769 and issue raised on #1735

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How has this been tested

  • go test ./... -race
  • golangci-lint run

gbjk added 2 commits January 9, 2025 15:55
Because this isn't really a file, it's a byte slice
@gbjk gbjk added bug review me This pull request is ready for review labels Jan 9, 2025
@gbjk gbjk self-assigned this Jan 9, 2025
@gbjk gbjk force-pushed the bugfix/config_downgrade branch from df0de84 to 8457cb1 Compare January 9, 2025 11:03
@gbjk gbjk added reconstructing Based on PR feedback, this is currently being reworked and is not to be merged and removed review me This pull request is ready for review labels Jan 9, 2025
@gbjk
Copy link
Collaborator Author

gbjk commented Jan 9, 2025

Pulling out the os.Exit and replacing with testable pathway.
Will push early tomorrow.

@gbjk gbjk force-pushed the bugfix/config_downgrade branch from 8457cb1 to a8dbd89 Compare January 10, 2025 06:48
@gbjk gbjk force-pushed the bugfix/config_downgrade branch from a8dbd89 to 82174cf Compare January 10, 2025 06:49
@gbjk gbjk added review me This pull request is ready for review and removed reconstructing Based on PR feedback, this is currently being reworked and is not to be merged labels Jan 10, 2025
@gbjk
Copy link
Collaborator Author

gbjk commented Jan 10, 2025

That's better, and testable.
Ready for review.

@gbjk gbjk mentioned this pull request Jan 10, 2025
3 tasks
Copy link
Collaborator

@shazbert shazbert left a comment

Choose a reason for hiding this comment

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

Just one thing and a suggestion. Thanks you! 🙏

config/config.go Outdated Show resolved Hide resolved
cmd/config/config.go Show resolved Hide resolved
@gbjk gbjk requested a review from shazbert January 20, 2025 03:51
Copy link
Collaborator

@shazbert shazbert left a comment

Choose a reason for hiding this comment

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

Last thing on new changes and linter issue then looks good to go. Thanks!

config/versions/versions.go Outdated Show resolved Hide resolved
@gbjk
Copy link
Collaborator Author

gbjk commented Jan 20, 2025

Last thing on new changes and linter issue then looks good to go. Thanks!

Linter fixed :)

@gbjk gbjk requested a review from shazbert January 20, 2025 06:36
Copy link
Collaborator

@shazbert shazbert left a comment

Choose a reason for hiding this comment

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

tACK. Thank you.

@shazbert shazbert added the szrc shazbert review complete label Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug review me This pull request is ready for review szrc shazbert review complete
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Config Version Downgrade is irrational
2 participants