-
Notifications
You must be signed in to change notification settings - Fork 273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove auto-patcher #828
Comments
I think we should keep it -- it's nice -- nobody else is gonna run it -- I fixed it so doesn't screw things up too badly...I personally like it...but that's me. |
FYI -- Danger can do linting -- replacing coala entirely. |
As far as I know, danger doesnt have the auto-patching functionality that coala provides, which is being used by coala_patch.sh. Also danger doesnt have many linters, so switching to danger would be a regression. So I think danger is mostly unrelated to this issue. But, in case I have missed something, I have created coala/meta#65 to compare the two more thoroughly. This issue is about whether it is better for GCI participants to see their code style problems that need to be fixed, or auto-fix it for them. I believe the former is educational, and the latter not so much. Adding gitmate would make their code style problems appear as notes inline on the code of the PR, making it quite easy for them to see the problem and add gitmate even tells them what to change (i.e auto-fix-suggestion). |
The only things I've seen coala fix is trailing whitespace which isn't easy to see when they use the github UI to edit the files. That's automation that I don't mind. |
Also worth noting, that coala-patch.sh exists because it shouldn't actually act on pull requests...and I ensure it doesn't...it also doesn't even bother to do anything if no changes are found (incredibly simple check since most git commands have a |
@jayvdb sir coala just fixes minute errors like trailing whitespaces, new line at eof error and other simple errors ;) it can't fix id name issues nor other complicated ones it only applies the patches for minute fixes and that really helps the new contributors... |
as @Abhi2424shek said -- most of the changes are trailing whitespace and spacing changes. I think it's fine to leave it. |
@jayvdb can we have your status. |
I vote leave it. Trailing whitespace is hard to see...so having this is
good.
…On Jan 6, 2017 11:53 PM, "Abishek V Ashok" ***@***.***> wrote:
@jayvdb <https://github.com/jayvdb> can we have your status.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#828 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABaMCOWkJ3j0DjUMv9Btxx8ZZPR9y0rks5rPxpJgaJpZM4LbSZW>
.
|
I think it's ok now. |
We would require more reviews pls ... |
I think it's ok -- The problems it fixed were problems. I ignored it where it was screwing things up. It works. |
@jayvdb i am sorry if it disturbs you but an updated status from you would be appreciated. |
I'm gonna close this -- as the autopatches are useful since edits are done via the Github editor... It's not always clear where trailing whitespace is. |
Continuing the conversation from #824 (comment) where @robbyoconnor asks "Should we remove the auto-patcher?"
IMO the auto-patcher is a terrible idea. We should use gitmate instead. I've been saying that since the beginning. But it will reduce the number of participants who complete the beginner tasks, as it will tell them how to improve their code, but will not do it for them.
It will result in higher standards of participation and lower workloads for mentors.
It will also more accurately reflect the real world, and be more comparable with other GCI organisations, where real software quality standards are enforced by bots.
The text was updated successfully, but these errors were encountered: