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

feat: Allow top-level repo level configs without projects defined #2853

Merged
merged 3 commits into from
Dec 23, 2022

Conversation

MattiasAng
Copy link
Contributor

@MattiasAng MattiasAng commented Dec 22, 2022

what

In #2300 support for parsing atlantis.yaml without projects defined was added, but it did not add support for handling the configuration itself.

This pull request adds support for parsing parallel_plan, parallel_apply and automerge and overriding the default values by specifying them in atlantis.yaml.

why

  • Allow top-level repo level configs without projects defined

references

This will enable top-level keys to be set in `atlantis.yaml` even when
using autoplanning strategy.
Signed-off-by: Mattias Ängehov <mattias.angehov@castoredc.com>
@MattiasAng MattiasAng requested a review from a team as a code owner December 22, 2022 15:48
@nitrocode nitrocode changed the title Allow top-level repo level configs without projects defined feat: Allow top-level repo level configs without projects defined Dec 22, 2022
@nitrocode
Copy link
Member

Very nice @MattiasAng !

cc: @csainty please review if you get a chance

automerge = repoCfg.Automerge
parallelApply = repoCfg.ParallelApply
parallelPlan = repoCfg.ParallelPlan
}
Copy link
Contributor

@krrrr38 krrrr38 Dec 22, 2022

Choose a reason for hiding this comment

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

looks good. I got that buildAllProjectCommands consider these changes in buildProjectCommandCtx. But plan uses specific buildPlanAllCommands method and it doesn't support this feature. So this PR changes buildPlanAllCommands 👌

BTW, events.DefaultAutomergeEnabled and raw.DefaultAutomerge are duplicated. How about changing them and removing events.DefaultAutomergeEnabled?

			automerge := raw.DefaultAutomerge
			parallelApply := raw.DefaultParallelApply
			parallelPlan := raw.DefaultParallelPlan

Copy link
Member

Choose a reason for hiding this comment

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

@MattiasAng would you consider the above changes before merging?

Copy link
Contributor

@csainty csainty left a comment

Choose a reason for hiding this comment

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

👍

@jamengual jamengual added the feature New functionality/enhancement label Dec 23, 2022
@nitrocode nitrocode added this to the 0.22.0 milestone Dec 23, 2022
Copy link
Contributor

@krrrr38 krrrr38 left a comment

Choose a reason for hiding this comment

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

#2853 (comment) is optional, so approved for now.

@nitrocode nitrocode merged commit 35cb9b8 into runatlantis:main Dec 23, 2022
@nitrocode
Copy link
Member

Thank you @MattiasAng ! This is a very helpful contribution!

@MattiasAng
Copy link
Contributor Author

You guys work fast! Sorry, didn't have time to fix the one comment but see it was quite minor. 🙂

Thanks for the fast merge!

@MattiasAng MattiasAng deleted the allow-repo-level-configs branch January 2, 2023 15:23
@nitrocode
Copy link
Member

Hmm @csainty and @MattiasAng, without digging through the code of both PRs, what is the major difference between this PR #2853 and PR #2300 ? They share a similar title.

@MattiasAng
Copy link
Contributor Author

MattiasAng commented Jan 6, 2023

@nitrocode The previous PR didn't actually parse the values from atlantis.yaml, it used the default global ones.

It actually confused me as well first when I couldn't get it to work. 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality/enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants