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

buildresult: replace serialization with an enum to simplify compatibility #218

Merged
merged 3 commits into from Sep 7, 2018

Conversation

LnL7
Copy link
Member

@LnL7 LnL7 commented Aug 10, 2018

This separates the serialized BuildResult type that requires backwards
compatibility to avoid breaking results received from older builders and
the LegacyBuildResult type use internally.

// as long as we can translate all enum variants.
match self {
BuildResult::Legacy { repo, pr, system, output, attempt_id, request_id, attempted_attrs, skipped_attrs, .. } =>
LegacyBuildResult {
Copy link
Member Author

@LnL7 LnL7 Aug 10, 2018

Choose a reason for hiding this comment

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

I can also clean this part up a bit if you prefer, didn't want to make a huge pr.

}

#[test]
pub fn test_no_status_compatibility() {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is tested by the the serialization now.

@LnL7 LnL7 force-pushed the build-result-enum branch 2 times, most recently from 96487bc to 49c8482 Compare August 14, 2018 21:50
@LnL7
Copy link
Member Author

LnL7 commented Aug 14, 2018

I added a V1 format with tagging and cleaned up the usage a bit.

…lity

This separates the serialized BuildResult type that requires backwards
compatibility to avoid breaking results received from older builders and
the LegacyBuildResult type use internally.
This drops the old build success format and makes the request_id
mandatory (was optional for compatibility).
With this builders should start producing messages that include a
version tag.
@grahamc grahamc merged commit de1b197 into NixOS:released Sep 7, 2018
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.

None yet

2 participants