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

change --PR to --higher-version-required #158

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

anxdpanic
Copy link
Member

I wasn't sure if a complete rename was wanted, so I left --PR usable so the transition to the new argument doesn't need to be immediate with the next release.

@Rechi
Copy link
Member

Rechi commented Feb 6, 2019

This is exactly what I had in mind. Thanks for also thinking about our current scripts and keeping the --PR argument functional for now.

Only thing missing is updating the parameter in some (example) configuration files, namely .travis-addon.yml.example, .travis-repo.yml.example and .vscode/launch.json.

@anxdpanic
Copy link
Member Author

Updated those files now. .travis-repo.yml.example has a line that is too long, will look at adding and testing newline addition to the script if required

@Rechi
Copy link
Member

Rechi commented Feb 6, 2019

pycodestyle doesn't check that file so all fine.

@razzeee
Copy link
Member

razzeee commented Feb 7, 2019

To be honest, I would prefere configuration by use-case not configuration by feature of the addon checker.

So having some prepared grouped features that get triggered by a arg would be best in my mind.

addon checker --mode=PR
addon checker --mode=REPO
addon checker --mode=ADDON

Just a quick brainstorm, there are probably better names/more modes

@anxdpanic
Copy link
Member Author

That seems like the opposite direction of #157
I think that would require 4 modes at least, ADDON being split into local filesystem and ci to account for stuff like gitignored files.

@Rechi
Copy link
Member

Rechi commented Feb 7, 2019

@razzeee what would you enable or disable in each of those modes?

The number of all combinations of --higher-version-required and --allow-folder-id-mismatch is only 4. I don't see how having 3 different modes would be simper, as you then don't have the obvious argument names anymore.
I do not expect that we add any further command line arguments for disabling features.

@razzeee
Copy link
Member

razzeee commented Feb 7, 2019

Yes, we might not expect it, but still we might have to do that at one point.

Far more important is that those arguments need input at least the whitelist/blacklist ones. So saying it's just four isn't really fair. So either everybody will just copy the default ones we provide or every addon author needs to set them up on their own.

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.

None yet

3 participants