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

Rename regexManagers to customManagers #19066

Closed
Tracked by #19289
rarkins opened this issue Nov 24, 2022 · 19 comments
Closed
Tracked by #19289

Rename regexManagers to customManagers #19066

rarkins opened this issue Nov 24, 2022 · 19 comments
Assignees
Labels
manager:regex priority-2-high Bugs impacting wide number of users or very important features status:in-progress Someone is working on implementation type:refactor Refactoring or improving of existing code

Comments

@rarkins
Copy link
Collaborator

rarkins commented Nov 24, 2022

Describe the proposed change(s).

  • rename regexManagers to customManagers
  • add a new field customType which defaults to regex
  • add migration code
@rarkins rarkins added priority-2-high Bugs impacting wide number of users or very important features type:refactor Refactoring or improving of existing code status:ready labels Nov 24, 2022
@rarkins
Copy link
Collaborator Author

rarkins commented Nov 24, 2022

Nest the existing regexManagers code under a regex/ subdirectory too and wrap them so that we're ready for new customTypes

@RahulGautamSingh
Copy link
Collaborator

RahulGautamSingh commented Dec 8, 2022

  1. Should we also rename manager from regex to custom and have regex as customType only?

  2. How should we handle the logic to selet the which extract fn to use among custom managers when
    we have something like this in future:

    • module/managers
      • custom
        • regex
        • jsonata

    a) Add index.ts and api.ts files in custom folder and replicate the logic which is used for managers.

    OR

    b) When manager is regex use customType to select the correct manager api to use.
    like this:

    Currrent:

    const m = managers.get(manager)!;
    return m.extractPackageFile
    ? m.extractPackageFile(content, fileName, config)
    : null;
    }

    New:

	 	let m: ManagerApi;
  		if (manager === 'regex') {
    	m = managers.get(config.customType ?? 'regex')!;
 		 } else {
   		 m = managers.get(manager)!;
 		 }

@rarkins
Copy link
Collaborator Author

rarkins commented Dec 8, 2022

  1. Yes
  2. Wrap/duplicate the functions a little

@RahulGautamSingh
Copy link
Collaborator

RahulGautamSingh commented Dec 8, 2022

After shifting from regex to custom, how should we handle the enabledManagers option when we need to enabled regex or jsonaata in future?

  1. enabledManagers: [regex, jsonaata]
  2. enabledManagers: [custom]

I prefer 1) as it gives more control to user.

@viceice
Copy link
Member

viceice commented Dec 8, 2022

use 2.

@rarkins rarkins added status:in-progress Someone is working on implementation and removed status:ready labels Dec 16, 2022
@reitzig
Copy link

reitzig commented Apr 12, 2023

Will there be any API and/or implementation changes? (Sorry if the answer is obvious, didn't check the PR.)

FWIW, I was prompted to share thoughts on how regexManagers could be made easier to debug: renovatebot/renovate#20881

enabledManagers: [regex, jsonaata]

Are there plans to include one for XML? I felt rather dirty regexing into a Maven pom.xml.

@RahulGautamSingh
Copy link
Collaborator

RahulGautamSingh commented Jul 15, 2023

We might need to migrate matchManagers as well.
I prefer to keep it regex because if we fo with custom then, incase we have multiple custom managers all will be matched.
ie. no way to select only jsonata or regex.

@rarkins
Copy link
Collaborator Author

rarkins commented Jul 15, 2023

Support both then. custom will match any manager while regex matches only Regex, jsonata only jsonata, etc

@RahulGautamSingh
Copy link
Collaborator

Support both then. custom will match any manager while regex matches only Regex, jsonata only jsonata, etc

After discussions in #23357 (comment), we have decided to support the custom* syntax,
Example:

matchManagers = ['custom.regex', 'custom.jsonata']  // custom managers separately
enabledManagers= ['custom.*'] // all custom managers

@RahulGautamSingh
Copy link
Collaborator

RahulGautamSingh commented Aug 5, 2023

Doables in order:

Enhancement not directly related to the implementation

@RahulGautamSingh
Copy link
Collaborator

RahulGautamSingh commented Sep 2, 2023

We will need separate interfaces for CustomManager , RegexManager ...
CustomManager : will contain the common fields that are expected in all custom managers eg. customType, fileMatch, and RegexManager : will contain the fields which are unique to it like matchStrings , matchStringsStrategy & its templates

Similarly we will need to handle validation based on customType as well , after validating the customType and fileMatch fields.
For eg.

 if (key === 'regexManagers') {
 ...
                } else if (is.nonEmptyString(regexManager.customType)) {
                  if (is.nonEmptyArray(regexManager.fileMatch)) {
                    // TODO: Add validation as per customType, preferrably a separate file or function for readability
                    switch (regexManager.customType) {
                      case 'regex':
                        errors.push(
                          ...(validateRegexManagerFields(
                            regexManager,
                            currentPath
                          ) as any)
                        );
                        break;
                    }
                  } else {
                    errors.push({
                      topic: 'Configuration Error',
                      message: `Each Regex Manager must contain a non-empty fileMatch array`,
                    });
                  }
                } else {
                  errors.push({
                    topic: 'Configuration Error',
                    message: `Each Regex Manager must contain a non-empty customType string`,
                  });
                }
              }
            }
... 

Hence, when a new custom manager is added, we will need:

  1. a new interface and
  2. a new validation function to validate its fields and return errors

@RahulGautamSingh
Copy link
Collaborator

Should we rename RegexManagerPresets to CustomManagerPresets ? It makes sense, since in future we might include presets of other custom managers to the same list.

@RahulGautamSingh
Copy link
Collaborator

Doables in order:

Enhancement not directly related to the implementation

We can close this issue now, and transfer the enhancements ie. implementing custom.* syntax for enabledManagers and matchManagers to #21760

@TWiStErRob
Copy link
Contributor

Note: this is not mentioned in https://docs.renovatebot.com/release-notes-for-major-versions/ for v37

@rarkins
Copy link
Collaborator Author

rarkins commented Nov 12, 2023

@TWiStErRob any thing with migrations is not treated as a breaking change

@TWiStErRob
Copy link
Contributor

@rarkins I see it's not breaking, but could still worth a mention in commentary as it's a concept change?

@rarkins
Copy link
Collaborator Author

rarkins commented Nov 12, 2023

We document changes in whatever release they fall in

@TWiStErRob
Copy link
Contributor

The last PR that was the actual rename (#24451) was landed in 36.107.0, does that not make it part of major v37?

@rarkins
Copy link
Collaborator Author

rarkins commented Nov 12, 2023

Yes, just like the 106 features released before it in v36. We can't mention them all in v37

suzuki-shunsuke added a commit to aquaproj/aqua-renovate-config that referenced this issue Dec 8, 2023
* fix: migrate regexManagers to customManagers

- renovatebot/renovate#24451
- renovatebot/renovate#19066

* fix: set `"customType": "regex"`

* fix: set "customType": "regex",

* fix: set customType

* fix: set customType
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
manager:regex priority-2-high Bugs impacting wide number of users or very important features status:in-progress Someone is working on implementation type:refactor Refactoring or improving of existing code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants