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

Switch to using stream-metadata-go library #14

Merged
merged 3 commits into from
Jan 13, 2021

Conversation

cgwalters
Copy link
Member

After we have a shared Go library for streams, we can use it
in e.g. coreos-assembler/mantle as well as openshift-install.

@cgwalters
Copy link
Member Author

cgwalters commented Dec 18, 2020

I moved most of the metadata definitions here to https://github.com/cgwalters/stream-metadata-go per discussion in coreos/fedora-coreos-tracker#98 (comment)

So if this PR and approach looks good I propose we move github.com/cgwalters/stream-metadata-go into coreos/ and then look to merge this!

@bgilbert
Copy link
Contributor

I'm in favor. If we're moving these structs out to their own library, though, I think we should probably clean up the struct naming. It isn't very consistent with the terminology used elsewhere.

Also, maybe we should use a map of platforms rather than hardcoding the platform list into the struct? As is, we'll need to revendor the library every time we add a platform.

@cgwalters
Copy link
Member Author

I think we should probably clean up the struct naming. It isn't very consistent with the terminology used elsewhere.

Do you have a specific example? Actually if you want feel free to just push code to master in https://github.com/cgwalters/stream-metadata-go - I just invited you as a collaborator. (Or do PRs as you prefer)

@cgwalters
Copy link
Member Author

Also, maybe we should use a map of platforms rather than hardcoding the platform list into the struct? As is, we'll need to revendor the library every time we add a platform.

Some of them need special handling (mostly the public cloud ones like AWS/Azure/GCP) - but maybe the ones that are just "an image file" could be part of a string map I guess?

@bgilbert
Copy link
Contributor

Okay, I've pushed a bunch of refactoring to that repo.

One open question: calling the package stream is a bit of a misnomer, since it also includes structs for release metadata. Should we move stream.go and release.go into their own subpackages, and/or rename the package and repo e.g. to coreos-metadata-go?

(eek, coreos-metadata.)

@cgwalters
Copy link
Member Author

One open question: calling the package stream is a bit of a misnomer, since it also includes structs for release metadata.

Yeah, separate packages seems cleaner I guess.

@cgwalters
Copy link
Member Author

@cgwalters cgwalters force-pushed the use-lib branch 3 times, most recently from 3ee44cc to 57c4a9b Compare January 12, 2021 20:17
@cgwalters
Copy link
Member Author

OK this should be close to ready, just have the two dependency PRs outstanding.

@cgwalters cgwalters marked this pull request as ready for review January 12, 2021 22:18
@cgwalters
Copy link
Member Author

OK lifting draft!

main.go Show resolved Hide resolved
Since we'll still use it after switching to the coreos/stream-metadata-go
library.
We aren't shipping this right now in FCOS.
Prep for using coreos/stream-metadata-go.
main.go Outdated Show resolved Hide resolved
@cgwalters cgwalters enabled auto-merge (rebase) January 13, 2021 13:49
@cgwalters cgwalters merged commit 3578d6b into coreos:master Jan 13, 2021
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.

2 participants