Skip to content
This repository was archived by the owner on Apr 4, 2025. It is now read-only.

fix(@schematics/angular): fix missing link attribute for PWA#129

Closed
markgoho wants to merge 7 commits intoangular:masterfrom
markgoho:patch-1
Closed

fix(@schematics/angular): fix missing link attribute for PWA#129
markgoho wants to merge 7 commits intoangular:masterfrom
markgoho:patch-1

Conversation

@markgoho
Copy link
Copy Markdown
Contributor

@markgoho markgoho commented Sep 4, 2017

This change fixes a security/performance issue in links that open in new windows, and by the same token, increases the lighthouse score for PWAs. Assuming we (eventually) want a CLI-generated Angular project to be 100% PWA, this is a necessary change.

image

This change fixes a security/performance issue in links that open in new windows, and by the same token, increases the lighthouse score for PWAs. Assuming we (eventually) want a CLI-generated Angular project to be 100% PWA, this is a necessary change.
@filipesilva
Copy link
Copy Markdown
Contributor

Heya, can you change the commit message to fix(@schematics/angular): fix missing link attribute for PWA please? That's what's failing the CI. Other than that LGTM.

@markgoho markgoho changed the title [template]: fix missing link attribute for PWA fix(@schematics/angular): fix missing link attribute for PWA Sep 5, 2017
@markgoho
Copy link
Copy Markdown
Contributor Author

markgoho commented Sep 5, 2017

Updated title, sorry about that @filipesilva -- thanks for noticing this PR so quickly!

@filipesilva
Copy link
Copy Markdown
Contributor

@markgoho what I actually meant is the commit itself though, not the PR title. You can rename it by doing git commit --amend.

@markgoho
Copy link
Copy Markdown
Contributor Author

markgoho commented Sep 5, 2017

Okay I did git commit --amend -m "fix(@schematics/angular): fix missing link attribute for PWA" and then pushed. Let me know if that made it right.

@filipesilva
Copy link
Copy Markdown
Contributor

You got the right message, but now you have 3 commits, some of which have invalid messages.

A good way of squashing them all into one is to use git rebase master -i. On the first commit replace the pick word with reword and on the other three replace pick with fixup. This will give you a single final commit.

@markgoho
Copy link
Copy Markdown
Contributor Author

markgoho commented Sep 5, 2017

Okay, I thought I rebased it properly, but it looks like CI is failing again :(

@markgoho
Copy link
Copy Markdown
Contributor Author

markgoho commented Sep 5, 2017

Here's what my text editor looks like just before I hit save
image
Maybe fixup isn't the right keyword?

@filipesilva
Copy link
Copy Markdown
Contributor

Something has gone wrong, and I don't really know what it was :(

In my editor, rebase -i looks like this:
image

So I think fixup is right... but something obviously did not go according to plan. It seems a bunch of merge commits found their way in?

I think at this point it's easier to make a new PR since your changes are limited to a single file. I'm sorry this process is being so dragged out, and thanks for sticking through it!

@markgoho
Copy link
Copy Markdown
Contributor Author

markgoho commented Sep 7, 2017

Thanks for all of your effort too! I'll close this and re-open with the correct commit message :)

@markgoho markgoho closed this Sep 7, 2017
@markgoho markgoho deleted the patch-1 branch September 7, 2017 18:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants