Closed Bug 1532937 Opened 5 years ago Closed 5 years ago

Enable ESLint for dom/flex and dom/grid

Categories

(Core :: CSS Parsing and Computation, enhancement, P3)

Product:
Core
Component:
CSS Parsing and Computation
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla69
Tracking Flags:
Tracking Status
firefox68 --- wontfix
firefox69 --- fixed

People

(Reporter: standard8, Assigned: shashankaushik100, Mentored)

References

Details

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

Attachments

(2 files, 6 obsolete files)

Bug 1532937 - Enable ESLint for dom/flex and dom/grid (automatic changes). r?Standard8
47 bytes, text/x-phabricator-request
Details | Review
Bug 1532937 - Enable ESLint for dom/flex and dom/grid (manual changes). r?Standard8
47 bytes, text/x-phabricator-request
Details | Review

As part of rolling out ESLint across the tree, we should enable it for dom/flex and dom/grid.

To help Mozilla out with this bug, here's the steps:

  1. Comment here on the bug that you want to volunteer to help. I (or someone else) will assign it to you.
  2. Download and build the Firefox source code, see the docs for details. An artifact build is all you need.
    • If you have any problems, please ask on IRC in the #introduction channel. They're there to help you get started.
    • You can also read the Developer Guide, which has answers to most development questions.
  3. Start working on this bug
    • Please note:
      • We want to end up with two separate commits. One with the automatic changes, the second with the manual changes.
      • Although you'll change .eslintignore at the start, only the second commit should have the .eslintignore changes. Please follow the suggested commands closely.
    • Here's what to do:
      1. In .eslintignore, remove the dom/flex/** and dom/grid/** lines.
      2. Run eslint ./mach eslint --fix dom/
        • This should fix some of the issues.
      3. Inspect the diff to make sure that the indentation of the lines surrounding the changes look ok.
      4. Create a commit of the work so far. Note the extra dom/flex/ dom/grid at the end (this avoids committing .eslintignore at this stage)
        • $ hg commit -m "Bug nnn - Enable ESLint for dom/flex and dom/grid (automatic changes). r?Standard8" dom/flex dom/grid
      5. For the remaining issues, you'll need to fix them by hand. To find them, run ./mach eslint dom/.
        • Most of those should be reasonably easy to understand, but there's more information on some specific bits (especially no-undef) here and here.
      6. Create a second commit of the manual changes, note, there's no directory specifier this time, so .eslintignore will be included.
        • $ hg commit -m "Bug nnn - Enable ESLint for dom/flex and dom/grid (manual changes). r?Standard8
      7. Post the two commits via phabricator. Please use moz-phab to submit them.
  4. Once the patches are submitted, I'll take a look. If there's any changes necessary I'll comment in Phabricator, so be prepared to update the patches. If there's no changes, I'll request review from a dom peer, so there may still be more to go.
  5. Once we're happy with the changes, I'll push it to autoland - our integration branch. Your code will then soon be shipping to Firefox users worldwide!
  6. Now you get to think about what kind of bug you'd like to work on next. Let me know what you're interested in and I can help you find your next contribution.
Priority: -- → P3

Hi :)

I would like to work on this isuue?

Hi Sonia, please start work and feel free to ask if you have questions.

Assignee: nobody → soniasingla.1812

Sonia's not working on this at the moment, so opening it up.

Assignee: soniasingla.1812 → nobody

Hi Mark,

I would like to work on this bug. I am a first time contributor.
Please let me if this bug is already assigned.

Looking forward

Thanks,
Shashank Kaushik

hi, I would like to work on my very first bug..

Hi Shashank, thanks for the offer, please start working on it - I've just assigned it to you.

Vishal, I'm passing to Shashank as they got there first. I'd suggest you look at your existing bug 1547944 and try to sort out your problems there before taking on another one.

Assignee: nobody → shashankaushik100

Sure Mark. Setting up the Artifact build. Will keep you posted

Hi Mark,

Raised Pull request on github. (Automatic changes)
PR: https://github.com/mozilla/gecko-dev/pull/473
Please review.

Thanks,
Shashank Kaushik

Hi Shashank,

As described in comment0, you need to send patch via phabricator.

You can learn more about how to send patch here https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch

Let us know if you need any help :)

Flags: needinfo?(shashankaushik100)

Hi Shivam,

Thanks for sharing the Doc. I will go through it.

Is there any documentation on how one flavor is different from the other. how is gecko-dev different from mozilla-unified and some doc around how the code is structured between various layers (C++, JS etc.)

Thanks,
Shashank Kaushik

Flags: needinfo?(shashankaushik100)

Enable ESLint for dom/flex and dom/grid (automatic changes)

Attachment #9065861 - Attachment description: # Enter a commit message. Bug 1532937:Enable ESLint for dom/flex and dom/grid (automatic changes). r?Standard8 → Bug 1532937:Enable ESLint for dom/flex and dom/grid (automatic changes). r?Standard8
Attachment #9065861 - Attachment is obsolete: true

Hi Mark,

Is there a concept of branches with Mercurial.

  • If so how do i cut a feature/bug branch and raise PR/Revision to the master/default branch.
  • If not how to create independent revisions?

If you know some good resources of getting around Mercurial please share them.

Thanks,
Shashank Kaushik

Status: NEW → ASSIGNED

(In reply to Shashank Kaushik from comment #14)

Is there a concept of branches with Mercurial.

  • If so how do i cut a feature/bug branch and raise PR/Revision to the master/default branch.
  • If not how to create independent revisions?

If you know some good resources of getting around Mercurial please share them.

I think this might be a good document for you: https://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/workflows.html#to-label-or-not-to-label

Note that MQ is no longer supported now.

One other option you might be interested in is git-cinnabar it allows checking out a mercurial repository as a git repository. You'll still need to use moz-phab to submit patches rather than pushing them, but the workflow would otherwise be the same.

Did you see my comments in phabricator about making a few changes?

Flags: needinfo?(standard8)
Flags: needinfo?(standard8) → needinfo?(shashankaushik100)

Did you see my comments in phabricator about making a few changes?
Yes Mark i saw the comments. I was a little busy with some personal work, will take this up over the weekend.

Thanks for links. I will go over them.

Flags: needinfo?(shashankaushik100)

Depends on D31717

Depends on D32451

Hi Mark,

Changes done. Please review.

Thanks,
Shashank kaushik

Depends on D31717

Attachment #9067295 - Attachment is obsolete: true
Attachment #9067293 - Attachment is obsolete: true
Attachment #9067403 - Attachment is obsolete: true
Attachment #9065867 - Attachment is obsolete: true
Attachment #9065868 - Attachment is obsolete: true

Thanks Mark for the links. I now have a git-cinnabar version of the code.
Can i create multiple branches in git in my local and run "moz-phab" on each individually?.
I see in the above commit is says one is dependent on the other. I guess this happened because i raised them through the same branch.

(In reply to Shashank Kaushik from comment #23)

Thanks Mark for the links. I now have a git-cinnabar version of the code.

Thanks for the updates, I'll look at them in a moment. Just to note, if there are changes to do, then you'll need to commit amend the revisions, and then you can moz-phab submit again. As long as you don't loose the Differential Revision: information in the commit message, it will update the existing commits. moz-phab also indicates if it'll create a new or update an existing revision when it asks you if you want to submit.

Can i create multiple branches in git in my local and run "moz-phab" on each individually?.

Yes you can, by default it'll submit anything that goes back to central (or master, whatever the remote branch is called).

I see in the above commit is says one is dependent on the other. I guess this happened because i raised them through the same branch.

Yes, that's correct. We generally treat the commits as separate revisions, and they'll be landed in as separate commits as well.

Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7d65d293dc73
Enable ESLint for dom/flex and dom/grid (automatic changes). r=Standard8,smaug
https://hg.mozilla.org/integration/autoland/rev/eb279dbdefe4
Enable ESLint for dom/flex and dom/grid (manual changes). r=Standard8,smaug

Hi Mark,

I would like to work on https://bugzilla.mozilla.org/show_bug.cgi?id=1532933 once you feel this one is done.

Thanks,
Shashank

Yes, this is done, thank you.

https://hg.mozilla.org/mozilla-central/rev/7d65d293dc73
https://hg.mozilla.org/mozilla-central/rev/eb279dbdefe4

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox69: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla69

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

status-firefox68: --- → ?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: