1534285 - Make spacing between member functions in CacheIR.h consistent

Status: RESOLVED FIXED

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

Description

[Matthew Gaudet (he/him) [:mgaudet]]

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.

Comment 1

[Huang Li-Pang]

Hi Matthew!

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

Comment 2

[Matthew Gaudet (he/him) [:mgaudet]]

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

Comment 3

[Huang Li-Pang]

Hi Mathew!

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

Comment 4

[Matthew Gaudet (he/him) [:mgaudet]]

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

Comment 5

[Karan Sapolia]

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

Comment 6

[Iain Ireland [:iain]]

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.

Comment 7

[Karan Sapolia]

Attachment: Bug 1534285 - Make spacing between member functions in CacheIR.h consistent. r=mgaudet

Comment 8

[jyotindrsingh]

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

Comment 9

[Karan Sapolia]

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

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/69309ca1e67f
Make spacing between member functions in CacheIR.h consistent. r=mgaudet

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/69309ca1e67f