X Tutup
Skip to content

fix(benchpress): make benchpress fit for chrome 45#3982

Closed
goderbauer wants to merge 1 commit intoangular:masterfrom
goderbauer:benchpress-chrome-45
Closed

fix(benchpress): make benchpress fit for chrome 45#3982
goderbauer wants to merge 1 commit intoangular:masterfrom
goderbauer:benchpress-chrome-45

Conversation

@goderbauer
Copy link
Copy Markdown
Contributor

@goderbauer goderbauer commented Sep 3, 2015

Closes #3411

tbosch
tbosch reviewed Sep 3, 2015
View reviewed changes
modules/benchpress/src/webdriver/chrome_driver_extension.ts Outdated
Copy link
Copy Markdown
Contributor

@tbosch tbosch Sep 3, 2015

Choose a reason for hiding this comment

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

We can have multiple categories per event. We could keep the string and create a helper method matchesCategory that checks via indexOf or a regex.

Copy link
Copy Markdown
Contributor Author

@goderbauer goderbauer Sep 3, 2015

Choose a reason for hiding this comment

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

I am not parsing the categories in a list and check them individually.

@goderbauer goderbauer force-pushed the benchpress-chrome-45 branch 5 times, most recently from 5f27116 to 1e4b25c Compare September 3, 2015 23:44
tbosch
tbosch reviewed Sep 4, 2015
View reviewed changes
modules/benchpress/src/webdriver/chrome_driver_extension.ts Outdated
Copy link
Copy Markdown
Contributor

@tbosch tbosch Sep 4, 2015

Choose a reason for hiding this comment

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

Could you use this approach everywhere where we check for the category as well? I.e. also above where we check for the disabled-by-default... category?

Copy link
Copy Markdown
Contributor Author

@goderbauer goderbauer Sep 4, 2015

Choose a reason for hiding this comment

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

Done.

@tbosch tbosch added pr_state: LGTM action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Sep 4, 2015
@tbosch tbosch assigned goderbauer and unassigned tbosch Sep 4, 2015
@goderbauer goderbauer force-pushed the benchpress-chrome-45 branch 2 times, most recently from c905e41 to ed1b54b Compare September 4, 2015 23:35
@goderbauer goderbauer added the action: merge The PR is ready for merge by the caretaker label Sep 5, 2015
@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Sep 5, 2015

User @goderbauer does not have PR merging privileges.

@mary-poppins mary-poppins removed the action: merge The PR is ready for merge by the caretaker label Sep 5, 2015
@tbosch tbosch added the action: merge The PR is ready for merge by the caretaker label Sep 5, 2015
@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Sep 5, 2015

Merging PR #3982 on behalf of @tbosch to branch presubmit-tbosch-pr-3982.

@mary-poppins mary-poppins removed the action: merge The PR is ready for merge by the caretaker label Sep 5, 2015
@goderbauer goderbauer closed this in 67b9414 Sep 5, 2015
@angular-automatic-lock-bot
Copy link
Copy Markdown

angular-automatic-lock-bot bot commented Sep 7, 2019

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

@goderbauer goderbauer

Labels

action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Benchpress does not work in Chrome 45 as the events changed

4 participants

@goderbauer @mary-poppins @tbosch @googlebot
X Tutup