search

node-dmx / dmx

DMX controller library for node.js
MIT License
297 stars 95 forks source link

added stop method to animation #30

Closed mStirner closed 6 years ago

mStirner commented 6 years ago

add new method to animation

Fensterbank commented 6 years ago

Hi @mStirner, could you please keep the original indentation?

You reformatted the whole file and changed the indentation from 4 to 2. Same thing with semicolons. In some lines you added semicolons, in other lines you didn't care about the missing semicolons.

These changes spams the file history, makes it difficult to see, what you really have done and in fact changing formatting has nothing to do with implementing the stop method. I think this is the reason why @wiedi did not merge this Pull Request.

Can you revert the formatting changes and update your branch? (you can use git commit --amend and git push --force)

Of course I also prefer an indentation of 2 and semicolons everywhere, but defining ESLint rules and reformatting the whole code using the rules should be done in another pull request.

Thanks and I'm looking forward for this PR to be merged! 👌

wiedi commented 6 years ago

Hi,

thanks for working on node-dmx!

There seems to be some issues with github not sending me notifications so I missed a lot of issues and PRs, sorry about this.

@Fensterbank is correct, having the actual change without the reformatting would help review a lot.

Fensterbank commented 6 years ago

I took your implementation while preserving the original formatting and force pushed to your branch.

Merged, thanks for your work! 🍪

@mStirner You should update your git config and setup your Github mail address. See here. At the moment your commits cannot be referenced to your Github profile because the mail address is set to marc@laptop-linux.