Closed Bug 1374827 Opened 7 years ago Closed 7 years ago

Remove em:unpack from the mozscreenshots extension

Categories

(Testing :: mozscreenshots, enhancement)

Product:
Testing
Component:
mozscreenshots
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox58 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla58
Tracking Flags:
Tracking Status
firefox58 --- fixed

People

(Reporter: MattN, Assigned: chochri5, Mentored)

References

(
URL
)

Details

Attachments

(1 file, 1 obsolete file)

Bug 1374827 - Remove lines dealing with em:unpack from mozscreenshots extension.
59 bytes, text/x-review-board-request
mconley
: review+
Details
Bug 1372696 is removing support for em:unpack and I'm not sure it's really needed for mozscreenshots from central anymore since we're currently using the screenshot binaries provided by automation (for better or worse).
Component: General → mozscreenshots
Product: Firefox → Testing
Version: Trunk → unspecified
Assignee: nobody → chochri5
Mentor: mconley, jaws
Status: NEW → ASSIGNED
Comment on attachment 8912080 [details] Bug 1374827 - Remove lines dealing with em:unpack from mozscreenshots extension. https://reviewboard.mozilla.org/r/183458/#review188626 ::: browser/tools/mozscreenshots/mozscreenshots/extension/install.rdf:14 (Diff revision 1) > > <!-- for running custom screenshot binaries --> > - <em:unpack>true</em:unpack> > + <em:unpack>false</em:unpack> Hi Chris, the default value is false when omitted so the tag should be deleted instead. You should also remove the comment above it since it's describing why `true` was necessary.
Comment on attachment 8912331 [details] Bug 1374827 - Removed lines dealing with em:unpack https://reviewboard.mozilla.org/r/183666/#review188904 Thanks Chris! This looks great to me, but can you try folding these two commits together? You'll need to use the `histedit` Mercurial extension to do that. `histedit` should have been enabled when you ran `./mach mercurial-setup`. Assuming you have no uncommitted changes, and you have the "Removed lines dealing with em:unpack" commit checked out, I think you should be able to do: `hg histedit -r bc631c966688` to bring up the vim interface for picking, dropping, re-ordering, and merging commits. This is very similar to `git rebase --interactive`. What you'll want to do is fold the second commit into the first by changing its first term from `pick` to `fold`. Once that's done, exit the editor (hopefully you're okay with exiting Vim - If not, ping me in IRC, and I'll walk you through it). You'll probably then get Vim again, asking you to update the commit message for the now-merged commits. Maybe make it something like: `Bug 1374827 - Remove lines dealing with em:unpack from mozscreenshots extension. r?mconley`.
Attachment #8912331 - Flags: review?(mconley) → review-
Comment on attachment 8912080 [details] Bug 1374827 - Remove lines dealing with em:unpack from mozscreenshots extension. https://reviewboard.mozilla.org/r/183458/#review188908
Attachment #8912080 - Flags: review?(mconley)
Attachment #8912331 - Attachment is obsolete: true
Comment on attachment 8912080 [details] Bug 1374827 - Remove lines dealing with em:unpack from mozscreenshots extension. https://reviewboard.mozilla.org/r/183458/#review189226 ::: commit-message-9db1c:1 (Diff revision 2) > +Bug 1374827 - Change em:unpack value to 'false' r?mconley > + > +MozReview-Commit-ID: 7kSOhkx6idu > +*** > + > +Bug 1374827 - Remove lines dealing with em:unpack from mozscreenshots > +extension. r?mconley > + > +MozReview-Commit-ID: FlEkeeIUNsL Great! Are you able to do: `hg commit --amend` to update this commit message? It's folded two commit messages together. I suspect we'll want the commit message to look like this: ``` Bug 1374827 - Remove lines dealing with em:unpack from mozscreenshots extension. r?mconley ```
Attachment #8912080 - Flags: review?(mconley)
Comment on attachment 8912080 [details] Bug 1374827 - Remove lines dealing with em:unpack from mozscreenshots extension. https://reviewboard.mozilla.org/r/183458/#review188626 > Hi Chris, the default value is false when omitted so the tag should be deleted instead. You should also remove the comment above it since it's describing why `true` was necessary. Hey Chris, you'll also want to mark this issue as "Fixed", otherwise we can't autoland it.
Comment on attachment 8912080 [details] Bug 1374827 - Remove lines dealing with em:unpack from mozscreenshots extension. https://reviewboard.mozilla.org/r/183458/#review189308 ::: commit-message-9db1c:1 (Diff revision 3) > +Bug 1374827 - Remove lines dealing with em:unpack from mozscreenshots > +extension. r?mconley These should be on a single line. ::: commit-message-9db1c:5 (Diff revision 3) > +*** > + > +Bug 1374827 - Remove lines dealing with em:unpack from mozscreenshots > +extension. r?mconley > + > +MozReview-Commit-ID: FlEkeeIUNsL Can you please remove these lines as well?
Attachment #8912080 - Flags: review?(mconley) → review-
Comment on attachment 8912080 [details] Bug 1374827 - Remove lines dealing with em:unpack from mozscreenshots extension. https://reviewboard.mozilla.org/r/183458/#review190236 Looks great, thanks!
Attachment #8912080 - Flags: review?(mconley) → review+
Pushed by mconley@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/cecd6d07c8bf Remove lines dealing with em:unpack from mozscreenshots extension. r=mconley
https://hg.mozilla.org/mozilla-central/rev/cecd6d07c8bf
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: