Closed Bug 1115141 Opened 5 years ago Closed 5 years ago

Add software usage license to StringHelper comments

Categories

(Firefox for Android :: Testing, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 37

People

(Reporter: mcomella, Assigned: Skandan, Mentored)

References

Details

(Whiteboard: [lang=java][good first bug])

Attachments

(1 file, 4 obsolete files)

Files in our source tree contain a license describing their usage rights (e.g. [1]). The class StringHelper, located in mobile/android/base/tests/StringHelper.java [2], does not contain such a license. Copy it from [1] (or any other file) and paste in in StringHelper.

To start, set up a build environment - you can see the instructions here: https://wiki.mozilla.org/Mobile/Fennec/Android

If you need any help, you can reply to this bug, or feel free to message me on IRC - my nick is "mcomella" and you can find me in #mobile. If you need IRC setup instructions, see https://wiki.mozilla.org/IRC

Thanks and happy coding! ^_^

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/BrowserApp.java
[2]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/tests/StringHelper.java
Hi Michael. I'm new to Open Source and the Mozilla community. I'd like to take this up.
@Michael there are multiple .java files which dont have license description in tests folder so what we have to do is to add license in every file?
Hello folks, and welcome to Bugzilla! :)

Sai, you've been assigned - let me know if you need any help!

Surabhi, for the purposes of this bug, let's handle just StringHelper. However, to avoid a collision on the assignee, I filed bug 1115542 and assigned you to handle all the tests/*.java files except StringHelper.

Happy holidays! :D
Assignee: nobody → prathikcoding167
Status: NEW → ASSIGNED
Thanks Michael and same to u..:)
Firstly Happy holidays @Michael!  and thank you for the opportunity :) . The licence has been copied. Any link or help on how i should post the patch? I tried hg -g diff. Console gave the correct output. But I was unable to find the patch. Maybe I was looking in the wrong place?
Please tell me if this is the proper way and if there is anything I must do. A link,for ways to create patches, would be helpful anyway. :)
Comment on attachment 8541590 [details] [diff] [review]
Patch with license added to StringHelper file

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

When you want to feedback on your patch or have it reviewed, add a feedback or review flag respectively. You can find these options in the details screen next to the patch in the attachments list.

This patch looks pretty good! I think the only thing that needs to be done is adding a bug number to the bug comment, i.e. "Bug 1115141 - Add license to StringHelper.java.". You can use `hg qref -e` to do this.
Attachment #8541590 - Flags: feedback+
I'm hoping you're referring to comment number 7 in the patch? I think i did something different the first time. So started anew following the steps in 
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F
Uploaded the new patch. Please direct me if I've done something different from what was needed.
Attachment #8542452 - Flags: review?(michael.l.comella)
Attachment #8542452 - Flags: feedback?(michael.l.comella)
Attachment #8541590 - Attachment is obsolete: true
Comment on attachment 8542452 [details] [diff] [review]
(New Patch)-LIcense added to StringHelper.java

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

This patch doesn't look right - perhaps you have multiple patches in your queue and added your commits to the wrong one? You can run `hg qseries` to see a list of patches in the queue and `hg qapplied` to see applied patches.

If you do have multiple patches, I recommend running `hg qfold` to combine them. See `hg help qfold` for more information in its usage.

(In reply to Prathik Sai  [:Skandan] from comment #8)
> I'm hoping you're referring to comment number 7 in the patch?

What do you mean by this?

> I think i did something different the first time.

There are several ways of producing the output required to make a patch file - the instructions in the link I sent you are just one way. You can do whatever works for you.
Attachment #8542452 - Flags: review?(michael.l.comella)
Attachment #8542452 - Flags: review-
Attachment #8542452 - Flags: feedback?(michael.l.comella)
Yeah i did find other patches, sorry about that :) . I used hg qpop -a and then used hg qdelete to get rid of the unnecessary patches. Made sure that the uploaded patch is the only one that is in the series and the only one applied. While using hg qref -m last time (probably because of the multiple applied patches) i was getting nano editor asking me to save in a temporary file. But it seemed to work fine now.
Attachment #8542452 - Attachment is obsolete: true
Attachment #8542806 - Flags: review?(michael.l.comella)
Attachment #8542806 - Flags: feedback?(michael.l.comella)
Comment on attachment 8542806 [details] [diff] [review]
bug-1115141.patch (The only applied patch)

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

This still doesn't look quite right - if you look at the review, you'll see the "pre" version already has a license in StringHelper. Perhaps you ran hg commit instead? If so (use `hg log` with no patches applied to find out), you might be able to run `hg qimport -r tip` which takes the top commit and moves it into your mercurial queue (i.e. it's no longer a permanent commit). Be sure not to do this on any commit but your own!

This could be tricky, depending on the state of your repository - it might be a problem to solve on IRC.

Note that I will be away on PTO until 1/13 - you can flag mhaigh (he's a mentor on this bug) if you need any help between now and then.

By the way, you only need to flag me for either review or feedback, whether or not you think your patch is final, or you want some feedback on which direction to take respectively.
Attachment #8542806 - Flags: review?(michael.l.comella)
Attachment #8542806 - Flags: review-
Attachment #8542806 - Flags: feedback?(michael.l.comella)
Alternatively, maybe you upload the wrong patch by mistake? Take a look at the review page and match it to the output of `hg log -p` to be sure.
Attached patch bug-1115141.diff (obsolete) — Splinter Review
Made a new patch. Sorry for the trouble on the previos patches. Still trying to identify what went wrong then.
Attachment #8542806 - Attachment is obsolete: true
Attachment #8543140 - Flags: feedback?
Attachment #8543140 - Flags: feedback? → feedback?(mhaigh)
Comment on attachment 8543140 [details] [diff] [review]
bug-1115141.diff

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

Nice - the change looks good, as does the patch itself.  Don't worry about messing up the first couple of tries, there's a lot to learn here and so many places to mess up!

So, because this looks good to me and it's such a small change, I've actually given you an 'r+'.  Normally now you would now pull in any code which has changed between the time you committed this patch and now and submit a new, final, patch with a slight addition to the commit message as mentioned here: https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#Commit_Message_Conventions.  This allows us to see in the repo log who has reviewed the patch prior to being committed.  Because this patch is so simple, you're not going to need to pull and merge, but do please submit a new patch with the reviewer details at the end of the commit message (you can use either myself [mhaigh] or mike [mcomella]).

Once you've done this and submitted the new patch to this bug, add a value of 'checkin-needed' to the 'keywords' box at the top of the bug screen - this will alert to a sherrif that this bug needs to be checked in.
Attachment #8543140 - Flags: feedback?(mhaigh) → review+
I'm currently on a vacation. Will be back on Monday. Should be able to upload the final patch by then. Thanks for the encouragement!! :)
This is the final patch with the changes added in commit message. I didn't know whether or not to select checkin or set it for review again in the attachment. So I uploaded this bug and I'm leaving a comment here.
Attachment #8543140 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/b8bf3fc77437
Flags: in-testsuite-
Keywords: checkin-needed
Whiteboard: [lang=java][good first bug] → [lang=java][good first bug][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/b8bf3fc77437
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [lang=java][good first bug][fixed-in-fx-team] → [lang=java][good first bug]
Target Milestone: --- → Firefox 37
You need to log in before you can comment on or make changes to this bug.