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

New: Add minimum hint severity #3340

Merged
merged 4 commits into from Nov 18, 2019
Merged

Conversation

@sarvaje
Copy link
Member

sarvaje commented Nov 13, 2019

Pull request checklist

Make sure you:

For non-trivial changes, please make sure you also:

  • Added/Updated related documentation.
  • Added/Updated related tests.

Short description of the change(s)

Fix #3069
Fix #3181

Here are the extensions to do self hosting.

Chromium:
webhint-1.2.1.zip

Firefox:
webhint-1.2.1.zip

@sarvaje sarvaje requested review from antross, molant and utsavized as code owners Nov 13, 2019
@@ -14,6 +14,7 @@
],
"references": [
{ "path": "../hint" },
{ "path": "../parser-html" },

This comment has been minimized.

Copy link
@molant

molant Nov 13, 2019

Member

I'm going to update the references en master and push a change directly so we don't have this files modified in here

@molant
molant approved these changes Nov 13, 2019
Copy link
Member

molant left a comment

LGTM, don't forget to rebase so a few files in here disappear. Also, can you please put a screen capture of how it looks like?

@sarvaje sarvaje force-pushed the sarvaje:extension-minimum-severity branch from 478b0d7 to 6313ba7 Nov 13, 2019
@sarvaje

This comment has been minimized.

Copy link
Member Author

sarvaje commented Nov 13, 2019

Rebase done!

Here is the screenshot:
image

@molant

This comment has been minimized.

Copy link
Member

molant commented Nov 13, 2019

@sarvaje

This comment has been minimized.

Copy link
Member Author

sarvaje commented Nov 13, 2019

It is the default option.

@sarvaje

This comment has been minimized.

Copy link
Member Author

sarvaje commented Nov 13, 2019

@molant do you want this #3181 to be part of this PR too?

@molant

This comment has been minimized.

Copy link
Member

molant commented Nov 14, 2019

@sarvaje

This comment has been minimized.

Copy link
Member Author

sarvaje commented Nov 14, 2019

This is how the extension looks for now. This is only the first version :)

image

image

image

image

image

image

image

@molant

This comment has been minimized.

Copy link
Member

molant commented Nov 14, 2019

@sarvaje

This comment has been minimized.

Copy link
Member Author

sarvaje commented Nov 14, 2019

Here are the extensions to do self hosting.

Chromium:
webhint-1.2.1.zip

Firefox:
webhint-1.2.1.zip

@sarvaje

This comment has been minimized.

Copy link
Member Author

sarvaje commented Nov 14, 2019

Here is the new order:
image

@molant
molant approved these changes Nov 14, 2019
@molant

This comment has been minimized.

Copy link
Member

molant commented Nov 14, 2019

Need to do some self-hosting before merging but it LGTM

@molant

This comment has been minimized.

Copy link
Member

molant commented Nov 15, 2019

@sarvaje I'm seeing something weird on the extension and the CSS prefixes hint:

image

I'm not seeing any of the prefixes in the code snippets.

This is form analyzing https://github.com/webhintio/hint/pull/3345/files with the sideloaded extension in this thread?

Can you please take a look?

@sarvaje

This comment has been minimized.

Copy link
Member Author

sarvaje commented Nov 15, 2019

I see the same thing in a version without these changes, so it is not something related to this PR.

@sarvaje

This comment has been minimized.

Copy link
Member Author

sarvaje commented Nov 15, 2019

I think is related with getCSSCodeSnippet that only return the item affected. I will take a look to double check.

@sarvaje

This comment has been minimized.

Copy link
Member Author

sarvaje commented Nov 15, 2019

@molant I have double checked it and the method getCSSCodeSnippet is generating only that piece of code.

@molant

This comment has been minimized.

Copy link
Member

molant commented Nov 15, 2019

@sarvaje I've created #3347

Can you please take care of it when you have time?

@sarvaje sarvaje force-pushed the sarvaje:extension-minimum-severity branch from 9d269c9 to c8ec1ca Nov 15, 2019
@sarvaje

This comment has been minimized.

Copy link
Member Author

sarvaje commented Nov 15, 2019

This is how it looks after adding the icons:
image

image

@sarvaje

This comment has been minimized.

Copy link
Member Author

sarvaje commented Nov 15, 2019

Here are the new versions for self hosting:

Chromium:
webhint-1.2.1.zip

Firefox:
webhint-1.2.1.zip

@molant molant self-requested a review Nov 16, 2019
@molant
molant approved these changes Nov 16, 2019
@molant molant merged commit fb663cc into webhintio:master Nov 18, 2019
5 checks passed
5 checks passed
licence/cla Contributor License Agreement is signed.
Details
webhintio.hint Build #20191115.6 succeeded
Details
webhintio.hint (Linux_PR node_12_x) Linux_PR node_12_x succeeded
Details
webhintio.hint (Windows_PR node_12_x) Windows_PR node_12_x succeeded
Details
webhintio.hint (macOS_PR node_12_x) macOS_PR node_12_x succeeded
Details
@sarvaje sarvaje deleted the sarvaje:extension-minimum-severity branch Nov 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.