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

Feature/update deploytargetconfig task #78

Merged
merged 15 commits into from
Feb 27, 2024

Conversation

rashed-k
Copy link
Contributor

@rashed-k rashed-k commented Feb 20, 2024

As previously, any update in config related to weight, pull-request and deploytarget ID, the id was deleted and then recreated as a update process for deploy target config. However, this wasn't the case for any updated branch in config which added new deploy target config but did not delete the previous deploy target config IDs.

I have now added additional logic for deletion_required which checks if there is any branch from existing config that does not match the desired config should be in deletion_required.

Also, the unit testing file for deploytargetconfig is updated.

@rashed-k rashed-k requested review from steveworley and yusufhm and removed request for yusufhm and steveworley February 20, 2024 08:39
Copy link
Contributor

@yusufhm yusufhm left a comment

Choose a reason for hiding this comment

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

@rashed-k PR looks good overall. I just had a question on a piece of the logic.

api/plugins/action/deploy_target_config.py Outdated Show resolved Hide resolved
Copy link
Contributor

@yusufhm yusufhm left a comment

Choose a reason for hiding this comment

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

Thanks for those changes @rashed-k ; looks good to me!

@yusufhm yusufhm merged commit cdb077a into master Feb 27, 2024
6 checks passed
@yusufhm yusufhm deleted the feature/update-deploytargetconfig-task branch February 27, 2024 04:30
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