Smalltalk › Amber Smalltalk

Newbie questions regarding git pull request

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

2 messages Options

fefr

Newbie questions regarding git pull request

Hello,

after the very nice amber presentation at FOSDEM I finally got around to play with Amber.

As a total git newbie I am a little lost how to contribute fixes. As an example, I forked the master github into my own repository. Started the node-server and tried to fix issue 113 (https://github.com/NicolasPetton/amber/issues/113).

I created a TestCase in StringTest:

testAsArray

self assert: 'hello' asArray = #('h' 'e' 'l' 'l' 'o').

which failed.

After digging around I found wrong order of block parameters in

Array (class)>>withAll:

withAll: aCollection

| instance |

instance := self new: aCollection size.

aCollection withIndexDo: [:each :index | "<- was: :index :each"

instance at: index put: each].

^instance

I committed the source, and committed the changes to my github-fork:

https://github.com/fizfaz/amber/commit/91f054341db56999a658cb2a65f192c3596eff6b

And now I am a little bit lost. Is it ok to issue a pull request? According to the diff it seems like every line was changed. Is it better to change the lines directly in the .st-files?

Cheers,

Felix

Nicolas Petton

Re: Newbie questions regarding git pull request

On 25/02/12 17:05, Felix Franz wrote:

> Hello,
>
> after the very nice amber presentation at FOSDEM I finally got around to
> play with Amber.
> As a total git newbie I am a little lost how to contribute fixes. As an
> example, I forked the master github into my own repository. Started the
> node-server and tried to fix issue 113
> (https://github.com/NicolasPetton/amber/issues/113).
>
> I created a TestCase in StringTest:
>
> testAsArray
> self assert: 'hello' asArray = #('h' 'e' 'l' 'l' 'o').
>
> which failed.
>
> After digging around I found wrong order of block parameters in
> Array (class)>>withAll:
>
> withAll: aCollection
> | instance |
> instance := self new: aCollection size.
> aCollection withIndexDo: [:each :index |"<- was: :index :each"
> instance at: index put: each].
> ^instance
>
> I committed the source, and committed the changes to my github-fork:
>
> https://github.com/fizfaz/amber/commit/91f054341db56999a658cb2a65f192c3596eff6b
>
> And now I am a little bit lost. Is it ok to issue a pull request?
> According to the diff it seems like every line was changed. Is it better
> to change the lines directly in the .st-files?

That's very likely because of the recompiled JS files. What changed in
the .st files? If it's only your fix, you can send me a pull request.

Thanks for contribution :)

Nico

>
> Cheers,
>
> Felix
>
>

--
Nicolas Petton
http://nicolas-petton.fr