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

[Merged by Bors] - feat(cdk): supply arch tag on publish if not set #3080

Closed

Conversation

galibey
Copy link
Contributor

@galibey galibey commented Mar 16, 2023

The presence of arch tag is required for multi-arch packaging. If it is not specified in package-meta.yaml we supply it before publishing (to be precise, before the package assembling).

@galibey galibey self-assigned this Mar 16, 2023
@galibey galibey added the DX/CDK Connector Developer Kit label Mar 16, 2023
@galibey galibey added this to the 0.10.6 milestone Mar 16, 2023
@galibey galibey requested a review from digikata March 16, 2023 16:48
@@ -498,6 +507,27 @@ pub fn package_verify_with_readio<R: std::io::Read + std::io::Seek>(
Ok(())
}

fn augment_arch(package_meta: &mut PackageMeta, target: Option<&str>) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't augment_arch just accept a target instead of Option. Why call it at all if target is None? Or is this a useful pattern? Thinking about this because we may other optional package tags in the future...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, moving a if outside and making augment_arch accept target looks nicer. I changed it. Thanks.

@galibey galibey requested a review from sehz March 16, 2023 22:51
Copy link
Contributor

@digikata digikata left a comment

Choose a reason for hiding this comment

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

LGTM!

@galibey
Copy link
Contributor Author

galibey commented Mar 20, 2023

bors r+

bors bot pushed a commit that referenced this pull request Mar 20, 2023
The presence of `arch` tag is required for multi-arch packaging. If it is not specified in `package-meta.yaml` we supply it before publishing (to be precise, before the package assembling).
@bors
Copy link

bors bot commented Mar 20, 2023

Build failed:

@galibey
Copy link
Contributor Author

galibey commented Mar 20, 2023

bors retry

bors bot pushed a commit that referenced this pull request Mar 20, 2023
The presence of `arch` tag is required for multi-arch packaging. If it is not specified in `package-meta.yaml` we supply it before publishing (to be precise, before the package assembling).
@bors
Copy link

bors bot commented Mar 20, 2023

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title feat(cdk): supply arch tag on publish if not set [Merged by Bors] - feat(cdk): supply arch tag on publish if not set Mar 20, 2023
@bors bors bot closed this Mar 20, 2023
@galibey galibey deleted the feat/add-target-tag-on-cdk-publish branch March 20, 2023 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX/CDK Connector Developer Kit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants