additional safety checks for config version #716
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
artoonie
force-pushed
the
feature/issue-712_config-versions
branch
from
f57ee5b
to
3c9b6bd
Compare
artoonie
force-pushed
the
feature/issue-712_config-versions
branch
from
3c9b6bd
to
801d33c
Compare
HEdingfield
approved these changes
Jun 26, 2023
Comment on lines
65
to
67
| // In practice, this should never be thrown because we should have checked the version | ||
| // while loading the config, and the version number can never change after a load+migration. | ||
| // Still, as a safety check, we'll throw here. |
There was a problem hiding this comment.
I now understand why this was here... the blocking version check doesn't actually happen when loading the config, it happens when triggering validation. This check was designed specifically to warn CLI users... I just completely overlooked the comment at the top that says exactly this. I think we should basically revert this PR, but I'm going to tweak it a little bit just to avoid this confusion again in the future.
There was a problem hiding this comment.
Ah, I see -- the CLI cannot migrate the config, but the GUI does, because the GUI gives you an immediate representation of what it looks like migrated. This comment makes more sense now.
artoonie
added a commit
that referenced
this pull request
Jun 26, 2023
* additional safety checks for config version * Partial PR reversion and comments for clarity. --------- Co-authored-by: HEdingfield <hylton@groupagree.com>
artoonie
added a commit
that referenced
this pull request
Jun 27, 2023
* additional safety checks for config version * Partial PR reversion and comments for clarity. --------- Co-authored-by: HEdingfield <hylton@groupagree.com>
artoonie
added a commit
that referenced
this pull request
Jun 28, 2023
* WIP: editable tables * support aliases and fix provider * clean-up * swap transfer/vote% column, and by by fix (#720) * additional safety checks for config version (#716) * additional safety checks for config version * Partial PR reversion and comments for clarity. --------- Co-authored-by: HEdingfield <hylton@groupagree.com> * lock UI while edit in progress * Use pop-up for inline alias editing * add GUI label back for Provider * PR comments, linting * also disable all buttons * don't eat semicolons * look up disabled items via css selector * add instructions * Helper text tweaks. * Fix preceding whitespace before aliases --------- Co-authored-by: HEdingfield <hylton@groupagree.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #712
see commentary in Issue