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 branch state in url on regression page #608

Merged
merged 1 commit into from
Jan 27, 2018

Conversation

philpep
Copy link
Contributor

@philpep philpep commented Jan 17, 2018

Add a 'branch' parameter in the url so the branch selection can be
shared through the url.

Copy link
Collaborator

@pv pv left a comment

Choose a reason for hiding this comment

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

One comment, otherwise lgtm.

if (branches && branches.length > 1) {
current_title = "Regressions in " + branches[0] + " branch";
var info = $.asv.parse_hash_string(window.location.hash);
if (info.params.branch) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably no need to re-parse the location hash.
params is passed to this function (see line 45, 53)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, changed to use params directly.

@philpep philpep force-pushed the regresion-branch-url branch from 79c0845 to f7a405f Compare January 23, 2018 14:08
if (branches && branches.length > 1) {
current_title = "Regressions in " + branches[0] + " branch";
if (params.branch) {
branch_index = branches.indexOf(info.params.branch[0]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

info.params -> params

Add a 'branch' parameter in the url so the branch selection can be
shared through the url.
@pv pv force-pushed the regresion-branch-url branch from f7a405f to 452b786 Compare January 27, 2018 15:45
@pv pv merged commit 4112e36 into airspeed-velocity:master Jan 27, 2018
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 this pull request may close these issues.

2 participants