Relates to JIRA: FACTORY-5411
Did the tests run correctly for you? I tried (rebasing on master) and they fail. Could you please check?
If dependent_event is None, this is going to throw an exception.
What about the script for the migration? Shouldn't we have one to apply to changes in the database?
@jkaluza @cqi @qwan could one of you also review? Thanks.
@srieger Hi, could you please rebase this PR based on the latest master? Looks the codebase these changes are based on is out-of-date.
rebased onto 1b5c8a24ee363d8da339c57dced86d6dde098164
Sorry forgot to commit the migration file! Should be rebased now as well. The PR is a WIP because I was looking for some advice regarding the tests. It seems like they relate to the rpm package somehow, but I am confused how this relates to what I am doing. I will continue to investigate, but if anyone has any suggestions/pointers please let me know.
rebased onto 2d73e083f47c98cea58043ce926cb0075d12520a
@gnaponie @cqi Would you mind looking again now that the tests are fixed? Thanks!
we can remove these comments
Looks this piece of code is copied from _create_rebuild_event_from_request and modified for manual rebuild event specifically, that causes duplicate code.
_create_rebuild_event_from_request
Can we split the _create_rebuild_event_from_request firstly for manual rebuild event and async rebuild individually, and then modify the one for manual rebuild to add the necessary code for FACTORY-5411?
I'm confused with this name reused_from. If it is an event ID, could you rename it to including word "event", or something else as long as the name could reflect the meaning clearly and directly?
reused_from
On the other hand, I don't understand how this field could help to fulfill the AC mentioned in the card. Could you please elaborate?
The AC includes one item for changing state_reason, this PR has no code to change that. Is the AC mentioned in the card out-of-date?
state_reason
@srieger is there any update for this PR?
@gnaponie Hi! Sorry - I had been waiting for more reviewers and didn't realize that people had commented. Will look at this soon :)
Looks this piece of code is copied from _create_rebuild_event_from_request and modified for manual rebuild event specifically, that causes duplicate code. Can we split the _create_rebuild_event_from_request firstly for manual rebuild event and async rebuild individually, and then modify the one for manual rebuild to add the necessary code for FACTORY-5411?
@cqi What do you mean by split? Like make two methods or make an if-else type of thing?
if-else
I'm confused with this name reused_from. If it is an event ID, could you rename it to including word "event", or something else as long as the name could reflect the meaning clearly and directly? On the other hand, I don't understand how this field could help to fulfill the AC mentioned in the card. Could you please elaborate?
This is the event that the builds are being reused from, so I think renaming it to be more clear is a good idea. I think the original plan with this was to eventually reference it in the rebuild reason
What's the state of this PR? Can we resolve (either merge or close) this?
Closing since Sarah's is no longer working with us and she won't probably address these changes in the future. Who will pick up this work in the future can take inspiration by this PR.
Pull-Request has been closed by gnaponie
Relates to JIRA: FACTORY-5411