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

Cleanup PdfJsTelemetry addon stub #8254

Closed
wants to merge 2 commits into from
Closed

Conversation

georgf
Copy link
Contributor

@georgf georgf commented Apr 7, 2017

This is a follow-up to #8239, renaming the PdfJsTelemetry-addon.jsm file and removing left-overs.

@Snuffleupagus
Copy link
Collaborator

Unless I'm mistaken, it looks like the old extensions/firefox/content/PdfJsTelemetry-addon.jsm file isn't actually being removed in this PR (only a new file is added). Hence I'd suggest using the git mv command to rename the existing file, since this has the added bonus of retaining the commit history for the file.

@georgf
Copy link
Contributor Author

georgf commented Apr 7, 2017

I used git mv, is anything broken?

@Snuffleupagus
Copy link
Collaborator

I used git mv, is anything broken?

Well, not "broken" as such, but as mentioned it looks like the old PdfJsTelemetry-addon.jsm file isn't actually being removed in the patch!?

@timvandermeij
Copy link
Contributor

Indeed, something appears to have gone wrong. If you look at the diff at https://github.com/mozilla/pdf.js/pull/8254/files, PdfJsTelemetry-stub.jsm is created, but PdfJsTelemetry-addon.jsm still exists as well, while that one should have been removed.

@Snuffleupagus
Copy link
Collaborator

Landed in PR #8275 (obviously keeping the author information correct), thank you for the patch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants