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

Improve lint-chromium to output actionable information #16607

Merged

Conversation

Rob--W
Copy link
Member

@Rob--W Rob--W commented Jun 26, 2023

Before this commit, lint-chromium complained without an obvious course of action:

Warning: Pref objects doesn't have the same length.
Error: chromium/preferences_schema is not in sync

With this commit, the error message is more actionable:

Warning: extensions/chromium/preferences_schema.json does not contain entry for pref: enableFloatingToolbar
Error: chromium/preferences_schema is not in sync

@Rob--W
Copy link
Member Author

Rob--W commented Jun 26, 2023

Note: this is intentionally branched off an older version of the master branch that did not include #16605, to show the new effect in CI. The old result can be seen in https://github.com/mozilla/pdf.js/actions/runs/5369999621/jobs/9760078018?pr=16601 from PR #16601.

@Rob--W
Copy link
Member Author

Rob--W commented Jun 26, 2023

Note: this is intentionally branched off an older version of the master branch that did not include #16605, to show the new effect in CI.

Actually, CI doesn't show the error because gulp-chromium doesn't run in CI until #16601 lands. I've included the old and new warning messages in the commit and PR description for comparison.

In any case, with the fix from #16605 the lint-chromium output wouldn't fail anyway.

gulpfile.js Outdated Show resolved Hide resolved
@Rob--W Rob--W force-pushed the lint-chromium-actionable-warnings branch from 4e3b61c to 19084cf Compare June 27, 2023 22:46
Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment

Choose a reason for hiding this comment

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

r=me, with the comments addressed; thank you!

gulpfile.js Outdated Show resolved Hide resolved
Before this commit, lint-chromium complained without an obvious course
of action:

> Warning: Pref objects doesn't have the same length.
> Error: chromium/preferences_schema is not in sync

With this commit, the error message is more actionable:

> Warning: extensions/chromium/preferences_schema.json does not contain an entry for pref: enableFloatingToolbar
> Error: chromium/preferences_schema is not in sync
@Rob--W Rob--W force-pushed the lint-chromium-actionable-warnings branch from 19084cf to 8330390 Compare June 28, 2023 10:30
@timvandermeij timvandermeij merged commit d057cae into mozilla:master Jul 1, 2023
@timvandermeij
Copy link
Contributor

Good idea; thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants