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

Make policy checks its own apply requirement. #61

Merged

Conversation

nishkrishnan
Copy link
Contributor

Having this as it's own apply requirement ensures that a project can be applied if it has it's policies passing, independent of whether other projects fail the policy check or not.

Additionally, the error messaging is clearer here.

for _, globalReq := range globalApplyReqs {
for _, currReq := range r.ApplyRequirements {
if globalReq == currReq {
continue OUTER
Copy link

Choose a reason for hiding this comment

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

I'm trying to understand this part, doesn't this create infinite loop? This is basically a GOTO statement right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, no this just allows us to continue from the next iteration of the outer loop without all the ugly syntax of having a boolean to flag this and then checking that in the outer loop before appending the requirement to the result list.

@@ -85,6 +85,19 @@ func (g GlobalCfg) Validate() error {

func (g GlobalCfg) ToValid(defaultCfg valid.GlobalCfg) valid.GlobalCfg {
workflows := make(map[string]valid.Workflow)

applyReqs := defaultCfg.Repos[0].ApplyRequirements
Copy link

Choose a reason for hiding this comment

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

Let's maybe put a comment here noting that we always initialize global config with at least 1 repo.

@nishkrishnan nishkrishnan requested a review from msarvar April 5, 2021 23:43
@nishkrishnan nishkrishnan merged commit c41d89d into release-v0.17.0-beta-lyft.1 Apr 6, 2021
msarvar pushed a commit that referenced this pull request Apr 19, 2021
* Make policy checks its own apply requirement. (#61)

* Remove warning from docs.
@mikecutalo mikecutalo deleted the nish/policy-pass-apply-req branch June 8, 2023 18:29
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