ENT-12098: Added an initial implementation of an internal cfbs command to generate MPF release information #208
Conversation
… MPF release information Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
|
Thank you for submitting a PR! Maybe @craigcomstock can review this? |
olehermanse
left a comment
olehermanse
left a comment
There was a problem hiding this comment.
Looks good so far, some preliminary comments.
Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
craigcomstock
left a comment
craigcomstock
left a comment
There was a problem hiding this comment.
Cool. Noticed a few things with a cursory look. I didn't check the logic too carefully. I can make another pass if need be.
Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
There was a problem hiding this comment.
Great work 🚀 I see you still have some # TODO's left to address. I have some smaller nitpicks for you. But in general:
- It's nice if you try to avoid too many nested
if's. They tend to make it really hard to understand the code. But this is not always the case. - When you look for the masterfiles tarball URL, maybe it's possible to search for it, instead of hard coding where it is in the JSON? Maybe there is a common denominator you can look for? I'm not sure.
I tried running your program, but it hung. Or maybe it just takes a very long time? I did not try to figure out why.
Looking forward to re-review!
Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
larsewi
left a comment
larsewi
left a comment
There was a problem hiding this comment.
Thank you, this looks good to me 🚀
Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
…lity Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
…ut additional dependencies Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
larsewi
left a comment
larsewi
left a comment
There was a problem hiding this comment.
Great improvements 🚀 I have some minor comments and some open questions that I hope @olehermanse or @craigcomstock can answer
Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
…download, document `cfbs generate-release-information`, change git-checkout wording Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
larsewi
left a comment
larsewi
left a comment
There was a problem hiding this comment.
LGTM 🚀 @jakub-nt maybe you can squash the commits? @olehermanse do you want to have one last look before we hit the merge button?
olehermanse
changed the title
olehermanse
left a comment
olehermanse
left a comment
There was a problem hiding this comment.
👍 Looks good to me, merging, and we can always submit follow-up fixes in new PRs.
5 participants
No description provided.