Closed Bug 1466098 Opened 2 years ago Closed 2 years ago

Some test defaults are set twice

Categories

(Firefox Build System :: Task Configuration, task, trivial)

3 Branch
task
Not set
trivial

Tracking

(firefox62 fixed)

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: marco, Assigned: bhavesh1999)

References

Details

(Keywords: good-first-bug)

Attachments

(1 file, 2 obsolete files)

I would like to work on this as my first contribution. Can you please guide me where I can edit this file and where I can submit the changes?
Hi!  This will make a good practice bug for contributing a patch to the Firefox source code.

Marco has pointed to the file in his comment.  There are a few guides to getting started contributing to Firefox, such as
  https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction
note that for this particular bug, there's no need to build Firefox (although it doesn't hurt).  You can test your changes by running ./mach taskgraph full -- if that runs without error, it's probably OK.

I hope that helps!
Assignee: nobody → bhavesh1999
Thanks a lot! I have started building Firefox, though it will take some time(my internet connection at the moment is slow) and I will make the changes as soon as possible.
Attached patch bug1466098patch (obsolete) — Splinter Review
Removed test defaults set twice
Comment on attachment 8985056 [details] [diff] [review]
bug1466098patch

Review of attachment 8985056 [details] [diff] [review]:
-----------------------------------------------------------------

I forgot to mark it for review.
Attachment #8985056 - Flags: review?(mcastelluccio)
Attached patch test-defaults-removed (obsolete) — Splinter Review
Removed max-run-time test default removed twice too.
Attachment #8985302 - Flags: review?(mcastelluccio)
Attachment #8985056 - Flags: review?(mcastelluccio) → review+
Comment on attachment 8985302 [details] [diff] [review]
test-defaults-removed

Review of attachment 8985302 [details] [diff] [review]:
-----------------------------------------------------------------

Both patches look good, but could you remove the `instance-size` duplicate too and squash the patches into a single one?

The commit message should be changed too, it should be in the format "Bug BUG_NUMBER - DESCRIPTION OF CHANGES. r=REVIEWER". So in this case "Bug 1466098 - Remove test defaults that are set twice. r=marco".
Attachment #8985302 - Flags: review?(mcastelluccio) → review+
Attached patch bug1466098patchSplinter Review
Removed instance-size test default.
Merged all patches into one.
Attachment #8985313 - Flags: review?(mcastelluccio)
Comment on attachment 8985313 [details] [diff] [review]
bug1466098patch

Review of attachment 8985313 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!
Attachment #8985313 - Flags: review?(mcastelluccio) → review+
Attachment #8985056 - Attachment is obsolete: true
Attachment #8985302 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Keywords: checkin-needed
(In reply to Marco Castelluccio [:marco] from comment #9)
> Comment on attachment 8985313 [details] [diff] [review]
> bug1466098patch
> 
> Review of attachment 8985313 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks!

Thanks to you too for helping me out!
(In reply to Dustin J. Mitchell [:dustin] pronoun: he from comment #2)
> Hi!  This will make a good practice bug for contributing a patch to the
> Firefox source code.
> 
> Marco has pointed to the file in his comment.  There are a few guides to
> getting started contributing to Firefox, such as
>  
> https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction
> note that for this particular bug, there's no need to build Firefox
> (although it doesn't hurt).  You can test your changes by running ./mach
> taskgraph full -- if that runs without error, it's probably OK.
> 
> I hope that helps!

Thanks a lot for helping me out!
Pushed by ebalazs@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/66ad37417e47
Removed test defaults that are set twice. r=marco
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/66ad37417e47
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Keywords: good-first-bug
Version: Version 3 → 3 Branch
You need to log in before you can comment on or make changes to this bug.