Closed Bug 1534285 Opened 9 months ago Closed 5 months ago

Make spacing between member functions in CacheIR.h consistent


(Core :: JavaScript Engine: JIT, enhancement, P3, trivial)




Tracking Status
firefox69 --- fixed


(Reporter: mgaudet, Assigned: sapoliakaran, Mentored)


(Keywords: good-first-bug, Whiteboard: [lang=c++])


(1 file)

The spacing between member functions in CacheIR.h has gotten very inconsistent, with big blocks where there's no empty line between member functions, then the occasional gap.

While it saves vertical space to not have the newlines, and I can imagine there being a nice grouping aspect provided by injecting whitespace on occasion, today the file just takes extra mental effort to read and write that could be returned if we just unified on one line of whitespace between member functions.

Mentor: mgaudet

Hi Matthew!

I can do this!
I want to ask adding new line is for line 805 to line 859 or the whole file?

Hey -- I would do the whole file; the link there was just for demonstrating the problem.

Hi Mathew!

I have completed the task. How can I pull request by using git?

Hey! That's good news. Let's see if we can't figure out how to review and then land this patch.

For review, we don't use GitHub, we instead use a tool called Phabricator. Most Mozilla developers work using Mercurial, and so the official walkthrough of the process only talks about Mercurial.

The easiest road for you would be to install mercurial, export your patch from Git, and reimport into mercurial and submit from there -- My recollection being that the inital setup of git being a bit slow.

Having said that, there are a good number of Mozilla developers who work in Git, and so there is a workflow that uses git. I personally am much less familiar with it, but it appears that the GeckoView team has reasonably good documentation of what that looks like.

git-cinnabar is something you'll see mentioned: It's a tool which is able to communicate with mercurial repositories from Git

Hi Matthew,
If Huang is not currently working on this, may I take this up?

Hi Karan!

Matt is on PTO today, but the answer to your question is: yes, you can absolutely take this up.

Let me know if you need any help.

Is this issue still open? I'd like to contribute to this!

Matthew would you mind adding the checkin-needed keyword here. I can't edit the keywords it seems. Thank you :)

Keywords: checkin-needed

Pushed by
Make spacing between member functions in CacheIR.h consistent. r=mgaudet

Keywords: checkin-needed
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
Assignee: nobody → sapoliakaran
You need to log in before you can comment on or make changes to this bug.