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

Enable Go managers #98

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Enable Go managers #98

wants to merge 4 commits into from

Conversation

nvtnlucie
Copy link
Contributor

@nvtnlucie nvtnlucie commented Nov 15, 2024

2 Go managers are added in this commit. I also moved the branchPrefix attribute one level higher as it seems from the documentation the effect stays the same.

Closes CWFHEALTH-3173
Related PR: konflux-ci/mintmaker-renovate-image#71

},
"additionalBranchPrefix": "{{baseBranch}}/",
"branchPrefix": "konflux/mintmaker/",
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this change have an effect on tekton manager branches?

@gnaponie
Copy link
Contributor

gnaponie commented Nov 18, 2024

Perhaps it's just github diff changes that is a bit chaotic. But it seems like homebrew and osgi used to be explicitly enabled and not they aren't anymore. Any reason for removing these or is this a mistake?

@qixiang
Copy link
Contributor

qixiang commented Nov 18, 2024

I'd suggest splitting the additionalBranchPrefix + branchPrefix change into a separate PR, we can probably merge it first after checking the comments from Giulia and Lubomir.

We should test the go manager more carefully as this manager is a little complicated and users opened several requests regarding this manager (though they were opened against the other internal renovate instance).

@staticf0x
Copy link
Contributor

tekton manager needs this line added:

"additionalBranchPrefix": "",

because of this error:

E       AssertionError: assert 'konflux/references/main/main' == 'konflux/references/main'
E         
E         - konflux/references/main
E         + konflux/references/main/main
E         ?                        +++++

@staticf0x
Copy link
Contributor

Btw I think it's fine to keep all changes in this PR, but it really helps in this case to change GitHub's diff view to "split" (= side by side).

@nvtnlucie nvtnlucie force-pushed the golang-managers branch 2 times, most recently from d94540f to 05291d5 Compare November 29, 2024 13:34
@nvtnlucie
Copy link
Contributor Author

Thank you for the review, i split the change into 2 PRs, rebased and added schedules as well. can you take another look please?

@staticf0x
Copy link
Contributor

I tested gomod in three situations:

and all worked, but it needs the following config to work for all cases:

"gomod":
    {
        "postUpdateOptions":
        [
            "gomodUpdateImportPaths",
            "gomodTidy"
        ]
    },
    "packageRules":
    [
        {
            "matchManagers":
            [
                "gomod"
            ],
            "matchDepTypes":
            [
                "indirect"
            ],
            "enabled": true
        }
    ]

Copy link
Contributor

@staticf0x staticf0x left a comment

Choose a reason for hiding this comment

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

Looks good, tests passed, I think we can merge it if others agree.

@nvtnlucie nvtnlucie marked this pull request as draft December 4, 2024 13:48
@nvtnlucie
Copy link
Contributor Author

Converted to draft until optimizations are in place so we dont exceed renovate's runtime.

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.

5 participants