✨ Add versioning to election event configs#1855
Conversation
the version is being added at export level from .env file and is being checked at import. if system is in dev version not need to compare.
xalsina-sequent
left a comment
xalsina-sequent
left a comment
There was a problem hiding this comment.
Please review the comments and also add the formatting of the resulting json
| @@ -102,6 +102,7 @@ pub struct ImportElectionEventSchema { | |||
| pub reports: Vec<Report>, | |||
| pub keys_ceremonies: Option<Vec<KeysCeremony>>, | |||
| pub applications: Option<Vec<Application>>, | |||
| pub version: String, | |||
There was a problem hiding this comment.
This could make older election events import fails if version is not present. We could either add a default value to version or make the parameter optional for backwards compatibilit.
| @@ -445,13 +399,54 @@ pub async fn decrypt_document( | |||
| Ok(temp_file_path) | |||
| } | |||
|
|
|||
| fn extract_major(version: &str) -> Option<u32> { | |||
There was a problem hiding this comment.
This could give an error if the version string includes a 'v' character like v1.0.0.
| ); | ||
|
|
||
| // If current version is "dev", allow any import | ||
| if current_version == "dev" { |
There was a problem hiding this comment.
I would suggest to create a const &str for "dev", e.g. APP_VERSION_DEV and use it to replace all the ocurrences with "dev" in rust.
| major_str.parse::<u64>().ok() | ||
| } | ||
|
|
||
| fn check_version_compatibility(imported_version: &str, current_version: &str) -> Result<()> { |
There was a problem hiding this comment.
I would love to see unit tests for this fn and fn extract_major
There was a problem hiding this comment.
There could be a negative test to showcase when extract_major would return None because parse:: returns error. The AI can generate this quickly.
Also, if we follow a specific version convention defined somewhere (do we?) we should match this implementation to that.
3 participants
Parent issue: https://github.com/sequentech/meta/issues/6333