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

Add an option to disable or ignore specific linter errors when validating chrome extensions #67

Closed
MoralCode opened this issue Jun 23, 2023 · 3 comments · Fixed by #68
Closed

Comments

@MoralCode
Copy link
Contributor

This plugin provides a very convenient way to lint extensions when building. However, due to mozilla/web-ext#2532 it is inconvenient to use this for validating chrome extensions as the linting will essentially always fail

I think it would be nice to either:

  • have a flag that allows specific lint results (i.e. MANIFEST_FIELD_UNSUPPORTED with a message matching "/background" is in an unsupported format.) to be ignored when deciding whether to fail or succeed at linting
  • have a flag that treats all linter errors as warnings (similar to the existing flag, but the other way around)
  • have a flag to indicate this is being used to validate a chrome extension that will change this linting step's idea of a "supported format" to match chrome's

Hope this is an acceptable/supported usecase (if not let me know). Happy to make this a contribution as well once I know which option (if any) would be preferred for the codebase.

@MoralCode MoralCode changed the title Add an option to disable or ignore specific linter errors Add an option to disable or ignore specific linter errors when validating chrome extensions Jun 23, 2023
@birtles
Copy link
Collaborator

birtles commented Jun 24, 2023

Hi! Thanks for filing this.

Yes, those approaches sound reasonable. I wonder if a combination of approaches 1 and 3 would work? e.g.

  • First add a flag for disabling specific lint warnings
  • Then add another convenience flag to disable lint warnings known to be specific to Chrome etc.?

Presumably if both flags are set, the first one adds to the second one?

@MoralCode
Copy link
Contributor Author

MoralCode commented Jun 24, 2023

yeah that makes sense! Do you think something like that would be relatively easy for a first-time contributor to the codebase to implement? Any hints on what documentation I could start with?

@birtles
Copy link
Collaborator

birtles commented Jun 26, 2023

yeah that makes sense! Do you think something like that would be relatively easy for a first-time contributor to the codebase to implement? Any hints on what documentation I could start with?

Sorry for the delay!

I'm afraid I didn't write this project, I just maintain it, but it's a very very simple project. There are no tests and there is no build step. All you need to do is clone the project and run yarn install.

Then you will want to modified index.js, add the option for disabling specific lint warnings, then filter them out somewhere before here.

Finally, you will need to update index.d.ts to add the types for the new option.

Some day we should probably add some unit tests for this, e.g. using vitest. It's up to you if you want to do that now.

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 a pull request may close this issue.

2 participants