Skip to content

Conversation

karangoel16
Copy link

No description provided.

@ph4r05
Copy link
Owner

ph4r05 commented Apr 5, 2019

Hi! thanks for PR.
PLS remove .DS_Store and other nonrelevant files from the commit. Thanks!

@karangoel16
Copy link
Author

Sure on it

@karangoel16
Copy link
Author

Hi, I have removed all the unnecessary files from the pull request.

@ph4r05
Copy link
Owner

ph4r05 commented May 31, 2019

Ah sorry, I was quite busy lately. Could you pls squash all commits to one and force-push? Then we are done. Thanks!

.gitignore

v8 arch added

v8 arch added

v8 arch added

v8 arch added

compiled libraries

.DS_store and other unneccessary files have been removed
@karangoel16
Copy link
Author

done, you can squash and merge from your end as well, i believe

@ph4r05
Copy link
Owner

ph4r05 commented Jun 12, 2019

done, you can squash and merge from your end as well, i believe

Yeah sorry for that but I am just doing how I am used to from other opensource projects I participate on. It is quite often the case I cannot modify your code branch and the PR should be already in the form that no further modifications are needed from the authors.

Mainly due to the fact that such modifications would result in a bit of commit mess in the commit history and possibly rebase on the master branch - which sould be avoided. So it is usually PR author's responsibility to make the PR "clean".

There is also another new problem with the PR now. PRs should not contain merge commits. It creates another mess in the commit history. It is preferable to do the rebase of the PR branch on the master so history is linear.

Moreover, I've just noticed a new thing that there are binaries in the PR. I cannot merge that as I did not compile the binaries and I cannot verify it there is not a malicious code in it (as I provide them in my repo). I would rather recompile it myself when the PR is merged.

So now I can see two options, a) you update the PR so it contains only one commit with all the changes I mentioned, b) I will extract the changes you've made to a new commit, with your name in the commit message as an attribution and close this PR without merge. With b) you won't be stated as a contributor of the project, unfortunately. Or if there is an easy way to do this just let me know.

Thanks for your contribution, I really appreciate it. I just want to have a clean PRs. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants