Newbie - not able to add patch

[Kiran Sringeri]

I have to add a patch to https://github.com/gwtproject/gwt.

I tried to follow the steps mentioned in http://www.gwtproject.org/makinggwtbetter.html#submittingpatches.

But when I try to push my changes, I get the below error:

remote: Permission to gwtproject/gwt.git denied to MyLifeIsJava.

fatal: unable to access 'https://github.com/gwtproject/gwt/': The requested URL returned error: 403

Pushing to https://github.com/gwtproject/gwt

[Jens]

GWT is hosted at https://gwt.googlesource.com/gwt and only mirrored (read-only) to Github.

If you want to contribute you must push your patch to https://gwt.googlesource.com/gwt . To do so you need to signup to Gerrit, sign a CLA, obtain and install a HTTP password and finally install Gerrit's commit hook.

http://www.gwtproject.org/makinggwtbetter.html#checkingout

http://www.gwtproject.org/makinggwtbetter.html#submittingpatches

-- J.

[Thomas Broyer]

Hi there,

Actually, the GitHub repo is only a mirror; in http://www.gwtproject.org/makinggwtbetter.html#workingoncode we point to the "real" repo.

As you already cloned the repo, you can "fix" this using:

$ git remote set-url origin https://gwt.googlesource.com/gwt

A few notes based on your screenshot even before you try again to upload your patch:

I see you downloaded the commit-msg hook, but you might have done it "too late", as it didn't apply to your commit message (oh well, that window doesn't show the change being indexed/staged, so maybe you hadn't committed yet?). Amending your commit (from the Git GUI, check "amend previous commit", then commit again) should fix this.

Your commit message will be a bit cryptic when looked at a few weeks/months/years from now; I'd suggest something like:

   Remove coercion to null in Element#getPropertyObject

   Now that Boolean and Doubles are never boxed, the coercion to null

   meant that getPropertyObject will actually coerce a boolean false or

   number 0 or NaN to null rather than returning a java.lang.Boolean or

   java.lang.Double respectively.

   There's a small breaking change for empty-string values that would

   no longer be coerced to null.

   Bug: #9455

   Bug-Link: https://github.com/gwtproject/gwt/issues/9455

(note: I'm not extra-sure about the note on empty strings; do you think you could test this to confirm?)

Last, but not least, do you think you could add some tests to the ElementTest#testProperties? (using setPropertyObject/getPropertyObject with boolean true and false, double 0, NaN and a non-zero value –e.g. 42–, and an empty and non-empty strings)

If you don't find/know how to actually run the test, don't worry: push in the change, add a comment to the review (you'll get the direct link in the console after you push) that you didn't run the test, and I'll take care of verifying it.

[Thomas Broyer]

I had some time today (it's a day off in France), and I:

• confirmed the regression from 2.7
• confirmed the change in behavior with empty strings after the patch
• wrote the test (which is how I checked the above)

Patch ready for review at https://gwt-review.googlesource.com/17300