-
Notifications
You must be signed in to change notification settings - Fork 0
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
Fixed metadata names #39
Conversation
@g4s8 PLease assign to me, I'm available |
This pull request #39 is assigned to @paulodamaso/z, here is why; the budget is 15 minutes, see §4; please, read §27 and when you decide to accept the changes, inform @g4s8/z (the architect) right in this ticket; if you decide that this PR should not be accepted ever, also inform the architect; this blog post will help you understand what is expected from a code reviewer; there will be a monetary reward for this job |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@g4s8 PLease take a look at the comments. Also, don't forget to write the test cases for the new added methods, if any
* @param act The action to perform | ||
* @param temp The temp | ||
* @param act Action | ||
* @param repomd The temp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@g4s8 These paremeters could be better described, it is almost impossible to understand them without reading the entire method code
new Key.From("repodata/repomd.xml"), | ||
Files.readAllBytes(temp) | ||
) | ||
return Single.fromCallable(() -> Files.createTempFile(type, ".xml")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@g4s8 I think that you could leave an issue to fix this methods / architecture: currently both performUpdate
methods do a lot of thinks (creating files, zipping contents, saving file) and I think that these behaviors should be split into several methods
@paulodamaso I've added a puzzle to address you comments. Will merge this PR, since it's urgent a bit. |
@g4s8 Good to me, merge please |
Job was finished in 21 hours, bonus for fast delivery is possible (see §36) |
#29 - Changed
Repomd
implementation to generate metadata file names before updatingrepomd.xml
.