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(manager/copier): Implement manager #29215

Merged
merged 11 commits into from
Aug 6, 2024

Conversation

lkubb
Copy link
Contributor

@lkubb lkubb commented May 22, 2024

Changes

  • Adds support for updating Copier templates
  • Adds one self-hosted (copierTrust) and one repository configuration (copierOptions, with several children)

Context

Closes #25556

  • This implementation does not support git@github.com:some/repo-style URLs (I don't think they are used widely though)
  • It does not fail when the smart update algorithm results in merge conflicts, they are not always reliable. It adds a warning when a file is reported to have conflicts though.

Note: This is my first PR to a TypeScript project in general and I have limited prior experience with Renovate. I read the contribution guidelines and tried to follow existing examples, but copier is not a regular manager, so I hope this is how it should be done.

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

why do we need so many new config options? we don't have it for any other managers

@lkubb
Copy link
Contributor Author

lkubb commented May 22, 2024

why do we need so many new config options? we don't have it for any other managers

I'd argue that Copier is somewhat different from other managers because it can impact each file in the repository in relatively dynamic ways. Also honestly, I tend to lean towards providing more configurability to users when it does not add significant complexity, but am not married to most of the options.

I believe copierTrust (without this, some templates are unmanageable), data (for centrally updating some variables) and likely recopy (if merge conflicts should be avoided at all cost) are essential.

skip and exclude can help address specific annoying quirks (in some generated projects, example files are re-added even if they have been removed from the repository or can cause merge conflicts). dataFile and skipTasks are probably expendable and could be removed if we want to simplify the configuration.

@rarkins
Copy link
Collaborator

rarkins commented May 23, 2024

We need to go back to the discussion for all of this. It's undesirable to bloat the configs with so many new options so we should discuss them one by one and reach agreement chest.

@lkubb lkubb mentioned this pull request May 23, 2024
51 tasks
@lkubb
Copy link
Contributor Author

lkubb commented May 23, 2024

Please see my post in the discussion for a detailed context on each option.

(Sorry, posted in the issue first, corrected)

I have removed all Copier-specific options.

Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

lib/modules/manager/types.ts Outdated Show resolved Hide resolved
lib/modules/manager/copier/artifacts.ts Outdated Show resolved Hide resolved
lib/modules/manager/copier/artifacts.ts Outdated Show resolved Hide resolved
lib/modules/manager/copier/artifacts.ts Outdated Show resolved Hide resolved
lib/modules/manager/copier/artifacts.ts Outdated Show resolved Hide resolved
lkubb added 2 commits July 1, 2024 12:33
* correctly inquire for `allowScripts`
* respect `ignoreScripts`
* only log to debug
In case of simple renames, `git status` might detect it. It's not
reported in the other fields, so we need to account for that.
lib/modules/manager/copier/artifacts.ts Show resolved Hide resolved
lib/modules/manager/copier/artifacts.ts Outdated Show resolved Hide resolved
lib/modules/manager/copier/artifacts.ts Outdated Show resolved Hide resolved
lib/modules/manager/copier/artifacts.ts Outdated Show resolved Hide resolved
@lkubb lkubb requested a review from viceice August 5, 2024 11:11
lib/modules/manager/copier/artifacts.spec.ts Outdated Show resolved Hide resolved
lib/modules/manager/copier/artifacts.spec.ts Outdated Show resolved Hide resolved
lib/modules/manager/copier/artifacts.spec.ts Outdated Show resolved Hide resolved
lib/modules/manager/copier/artifacts.spec.ts Outdated Show resolved Hide resolved
lib/modules/manager/copier/artifacts.spec.ts Outdated Show resolved Hide resolved
@lkubb lkubb requested a review from viceice August 5, 2024 12:23
@rarkins rarkins added this pull request to the merge queue Aug 6, 2024
Merged via the queue into renovatebot:main with commit 70376cc Aug 6, 2024
38 checks passed
@renovate-release
Copy link
Collaborator

🎉 This issue has been resolved in version 38.19.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

copier support
4 participants