Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings
This repository was archived by the owner on Nov 6, 2023. It is now read-only.

WIP: Add script to trivialize some left-wilcard targets#17842

Closed
cschanaj wants to merge 3 commits into
EFForg:masterEFForg/https-everywhere:masterfrom
cschanaj:trivialize-wildcard-targetsCopy head branch name to clipboard
Closed

WIP: Add script to trivialize some left-wilcard targets#17842
cschanaj wants to merge 3 commits into
EFForg:masterEFForg/https-everywhere:masterfrom
cschanaj:trivialize-wildcard-targetsCopy head branch name to clipboard

Conversation

@cschanaj

@cschanaj cschanaj commented Apr 28, 2019 •
edited
Loading

Copy link
Copy Markdown
Collaborator

This script will trivialize at least 1.3k left-wildcard targets (nearly 1/3 of their population). In some sense, it seems too good to be true, hope I didn't mess up anything 😄

P.S. benchmark with memusage.js show the memory usage will be increased by 0.6 MB because of this change... if this is not justified, I will close this PR.

@pipboy96

pipboy96 commented Apr 28, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

@cschanaj This is justified since it reduces ruleset complexity and improves ruleset testing coverage, which are currently two very important tasks.

pipboy96
pipboy96 suggested changes Apr 28, 2019
View reviewed changes

@pipboy96 pipboy96 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm mostly concerned about flaky-looking for loops. They look like they may cause occasional wrong rewrites in some edge cases.

Comment thread utils/trivialize-rules/trivialize-targets.js Outdated
@pipboy96

pipboy96 commented Apr 28, 2019

Copy link
Copy Markdown
Contributor

Also, the actual package.json is missing, even though package-lock.json is there.

pipboy96
pipboy96 reviewed Apr 28, 2019
View reviewed changes
Comment thread utils/trivialize-rules/trivialize-targets.js Outdated
@pipboy96

pipboy96 commented Apr 28, 2019

Copy link
Copy Markdown
Contributor

Also makes sense to replace rules that are effectively equal to ^http: ➡️ https: after these changes. Will try to implement that.

@pipboy96 pipboy96 self-assigned this Apr 28, 2019
@cschanaj

cschanaj commented Apr 29, 2019

Copy link
Copy Markdown
Collaborator Author

Also makes sense to replace rules that are effectively equal to ^http: https: after these changes. Will try to implement that.

AFAIK, trivialize-rules.js already do part of this. It might be helpful if you would like to implement that yourself.

@pipboy96 pipboy96 mentioned this pull request Apr 30, 2019
Closed
@cschanaj cschanaj changed the title Add script to trivialize some left-wilcard targets WIP: Add script to trivialize some left-wilcard targets Apr 30, 2019
@cschanaj cschanaj closed this May 1, 2019
@cschanaj cschanaj deleted the trivialize-wildcard-targets branch May 1, 2019 08:03
@cschanaj cschanaj mentioned this pull request May 1, 2019
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
@pipboy96 pipboy96 pipboy96 requested changes
Reviewers whose approvals may not affect merge requirements

Assignees

@pipboy96 pipboy96

Labels

ruleset-enhancement toolchain

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@cschanaj @pipboy96
Morty Proxy This is a proxified and sanitized view of the page, visit original site.