Rename mfbt/unused.h to mfbt/Unused.h for consistency

RESOLVED FIXED in Firefox 51

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: kanru, Assigned: kanru)

Tracking

unspecified
mozilla51
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox51 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Assignee

Description

3 years ago
Since the object is renamed to Unused, it's logical to rename the header too. All other headers under mfbt are Capitalized and has matching class name.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Notes:
- These sort of things might require a clobber (renaming a file with a different case)
- These two patches should *not* be autolanded, but folded before landing.
Assignee

Comment 4

3 years ago
(In reply to Mike Hommey [:glandium] from comment #3)
> Notes:
> - These sort of things might require a clobber (renaming a file with a
> different case)

Thanks for the reminder. I'll change CLOBBER in case someone is building on case insensitive file system. Do we support that?

> - These two patches should *not* be autolanded, but folded before landing.

Yes, the split is only to make the review easier.
We don't have conditional clobbers.
Assignee

Comment 6

3 years ago
(In reply to Mike Hommey [:glandium] from comment #5)
> We don't have conditional clobbers.

I mean, do we support building on a case insensitive file system? I guess yes.
Considering Windows is case insensitive and you can't do anything about it, yes. Mac is also case insensitive by default.
hooray!
Blocks: 1297315

Comment 9

3 years ago
mozreview-review
Comment on attachment 8783804 [details]
Bug 1297276 - Rename mfbt/unused.h to mfbt/Unused.h for consistency.

https://reviewboard.mozilla.org/r/73484/#review71432
Attachment #8783804 - Flags: review?(nfroyd) → review+

Comment 10

3 years ago
mozreview-review
Comment on attachment 8783805 [details]
Bug 1297276 - Update all places using mozilla/unused.h

https://reviewboard.mozilla.org/r/73486/#review71434

r=me with Mike's caveats addressed when this patch lands.
Attachment #8783805 - Flags: review?(nfroyd) → review+
Comment hidden (mozreview-request)
Assignee

Updated

3 years ago
Attachment #8783805 - Attachment is obsolete: true

Comment 12

3 years ago
Pushed by kchen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/564549c354b0
Rename mfbt/unused.h to mfbt/Unused.h for consistency. r=froydnj
Assignee

Comment 13

3 years ago
I figured you might want to know this too.
Flags: needinfo?(jorgk)

Comment 14

3 years ago
Thanks, much appreciated.

Looks like we're not using it though:
https://dxr.mozilla.org/comm-central/search?q=unused.h&redirect=false
(I had it in a patch, but the reviewers made me take it out and replace it with real error checking.)
Flags: needinfo?(jorgk)

Comment 15

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/564549c354b0
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51

Comment 17

3 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/ec654ccc2e0f
Fix another instance of mozilla/unused.h that snuck in.
(In reply to Mike Hommey [:glandium] from comment #7)
> Considering Windows is case insensitive and you can't do anything about it,
> yes. Mac is also case insensitive by default.

This patch prevents people from doing |git pull -f| to get their head updated. I workaround it by |rm mtbf/unused*|. Do you think it make sense to send out a note to dev-platform and/or firefox-dev on that?
Flags: needinfo?(kchen)
Assignee

Comment 20

3 years ago
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #19)
> (In reply to Mike Hommey [:glandium] from comment #7)
> > Considering Windows is case insensitive and you can't do anything about it,
> > yes. Mac is also case insensitive by default.
> 
> This patch prevents people from doing |git pull -f| to get their head
> updated. I workaround it by |rm mtbf/unused*|. Do you think it make sense to
> send out a note to dev-platform and/or firefox-dev on that?

Thanks, done.
You need to log in before you can comment on or make changes to this bug.