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

Updated and synced feature lists and updated redirect links #198

Closed
wants to merge 39 commits into from

Conversation

UtiluMark
Copy link
Contributor

Updated and synced the feature lists in the README and the About window.
Updated the AMO links in the README and the About window to prevent
redirects.
Added link to Screengrab (fix version) add-on.
Part of #188

…extensions to the Clipboard"

This reverts commit 1bc25c1.
…on by always using spaces and never using tabs
Removal of unused files and fixed white-space issues (mozilla#188)
Updated and synced the feature lists in the README and the About window.
Updated the AMO links in the README and the About window to prevent
redirects.
Added link to Screengrab (fix version) add-on.
Part of mozilla#188
@UtiluMark
Copy link
Contributor Author

@whimboo I've replaced "titlebar" with "title bar" everywhere except in entity names and reverted the Copy about:support to Clipboard feature to Pastebin.
Please review and merge.

@@ -37,4 +40,5 @@ All bugs are reported to the Nightly Tester Tools component at bugzilla.mozilla.
This project uses [.editorconfig](http://editorconfig.org/#overview), which sets defaults for the formatting of the code. So enjoy the use of [compatible editor](http://editorconfig.org/#download). Just download and install the corresponding plugin.

# Related Add-ons:
* [Add-on Compatibility Reporter](https://addons.mozilla.org/en-US/firefox/addon/15003/)
* [Screengrab (fix version)](https://addons.mozilla.org/en-US/firefox/addon/screengrab-fix-version/)
* [Add-on Compatibility Reporter](https://addons.mozilla.org/en-US/firefox/addon/add-on-compatibility-reporter/)
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets keep the add-on compat reporter as first item so we have an alphabetical sorted list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@whimboo
Copy link
Contributor

whimboo commented Oct 21, 2015

Looks fine to me beside the nit. Once an update has been pushed and you (@xabolcs) are ok with the changes, can you please get it landed? Thanks.

@xabolcs
Copy link
Collaborator

xabolcs commented Oct 21, 2015

Once an update has been pushed and you (@xabolcs) are ok with the changes, can you please get it landed?

Sure!

@UtiluMark
Copy link
Contributor Author

@xabolcs Update done, so you can get this landed.

@xabolcs
Copy link
Collaborator

xabolcs commented Oct 21, 2015

@UtiluMark
Sorry for last minute review, but what are the benefits preventing redirects of AMO links?

@whimboo
Copy link
Contributor

whimboo commented Oct 21, 2015

FYI by default we squash all commits at the end and give the final commit a proper comment (please see git log). @UtiluMark maybe you can do it to make it easier for @xabolcs to get it landed.

@UtiluMark
Copy link
Contributor Author

@xabolcs The benefits of preventing redirects of AMO links are that the current links have more readable URLs (for example "nightly-tester-tools" in the URL is more descriptive compared to the number "6543" of the old URL).
The old AMO links are currently redirected to the new ones using a 301 MOVED PERMANENTLY redirect, these redirects might get removed at some point in the future since the URLs changed permanently, so the old AMO links might end up in 404 FILE NOT FOUND errors in the future.

@xabolcs
Copy link
Collaborator

xabolcs commented Oct 21, 2015

Thanks @UtiluMark!

@xabolcs
Copy link
Collaborator

xabolcs commented Oct 21, 2015

Looks good to me.

Ohh, almost forgot. @whimboo could you provide a commit message for this PR? :)
Thanks!

@whimboo
Copy link
Contributor

whimboo commented Oct 22, 2015

I would like to let @UtiluMark do the squashing of commits and the commit message as pointed out in my last comment. @UtiluMark do you think you can do it? If you need help with let us know. In general make use of git rebase -i master and fixup all commits except the first one which needs an updated message.

Replaced "titlebar" with "title bar" everywhere except in entity names + Copy about:support to Clipboard reverted to Pastebin

Lets keep the add-on compat reporter as first item so we have an alphabetical sorted list.

Updated the AMO links in the README and the About window to prevent redirects.
@xabolcs
Copy link
Collaborator

xabolcs commented Oct 22, 2015

@whimboo it's OK to me if you would like to teach @UtiluMark contributing to NTT, but a squashed commit of this PR is already in my working copy. It's OK to me to do the squashing work. :)

@UtiluMark
Copy link
Contributor Author

@xabolcs I've just done what @whimboo asked. If you already have a squashed commit thats fine. :)

@xabolcs
Copy link
Collaborator

xabolcs commented Oct 22, 2015

@UtiluMark, a git commit --amend is missing for me, to update the commit message in my copy.

@UtiluMark
Copy link
Contributor Author

@xabolcs Done, changed the commit message to "Updated and synced feature lists and updated redirect links".

@whimboo
Copy link
Contributor

whimboo commented Oct 22, 2015

There are still 39 commits listed on this PR. You might not have pushed via -f. Also for comments we add the issue number in brackets at the end.

@UtiluMark
Copy link
Contributor Author

So the commit message for this PR should be:

Updated and synced feature lists and updated redirect links (#198)

@xabolcs You wrote you already have a squashed commit of this PR, so there is nothing more to be done for me, right?

@xabolcs
Copy link
Collaborator

xabolcs commented Oct 22, 2015

@UtiluMark, sure, but @whimboo would like to let you do all the work. :)

The commit message is almost correct. It should refer to #188 for now.

@xabolcs
Copy link
Collaborator

xabolcs commented Oct 28, 2015

@UtiluMark, take the squash & rebase exercises on the next PR! :)

Merging this.

@xabolcs
Copy link
Collaborator

xabolcs commented Oct 28, 2015

Landed as commit f313b2b.

@xabolcs xabolcs closed this Oct 28, 2015
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.

3 participants