Closed CatalinStratu closed 11 months ago
@CatalinStratu here are my 2 cents advices:
As a base rule, always make limited changes when you are proposing a PR. Stay focused and avoid any changes not directly related to the problem at hand
Also you need to work on your commit messages: "Improvements" without a body and explanation is well... not really informative. It tells me zero thing about the PR
Do not change the .gitignore. Do a separate PR for this
Do not change their core code for no reason. None would ever accept this.
Only suggest changes that help you integrate as a library, not one extra thing
And you will need to sign the CLA.
Please keep in mind that submitting a PR needs to bring value and requires significant time from the maintainers to review and accept. So please be mindful and make sure you get things properly prepared before submitting a PR.
I would suggest that for now you close this PR, and work out something on your fork first.
So if this were a PR on one of my projects I would reject it outright. :)
@pombredanne @CatalinStratu thanks both for you discussion and suggested changes.
@CatalinStratu I agree with the need to keep PRs minimal and feature specific. If you'd like to contribute larger changes let's chat first so you don't accidentally do work I wouldn't be willing to merge :)
The main focus for this project is to mirror the upstream go internal code https://github.com/golang/go as much as possible. I've diverged already in some necessary ways for correct recovery for malicious binaries, but when possible we'd like to be 1-1 with what the upstream code does. This includes naming, comments, indents, etc. By doing this we ensure future maintence is easier as the upstream becomes a master copy we can reference - and far future perhaps GoReSym could exist as a .git patch on-top of upstream (probably dreaming about that)
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
View this failed invocation of the CLA check for more information.
For the most up to date status, view the checks section at the bottom of the pull request.