Show HN: Sedy, a GitHub Bot Bringing Linux Sed to Code Reviews(marmelab.com)
marmelab.com
Show HN: Sedy, a GitHub Bot Bringing Linux Sed to Code Reviews
https://marmelab.com/blog/2017/02/28/sedy-the-serverless-github-bot-which-fix-typos-for-you.html
44 comments
Regarding the first point, I think it may be useful to have it need to explicitly be summoned (`@sedy` or whatever), rather than having it try to guess which comments are intended for it. That would also make it simpler to support other bits of sed syntax (like alternate delimiters), that would be hard to auto-recognize as sed commands.
A good thought!
My point was orthogonal, though. I don't want a reviewer directly making committed changes to my code, period. Giving me control isn't about preventing inadvertent calls to sedy. It's about handling when someone suggests a substitution that I don't agree with, or want to reword.
My mindset might be different from what Marmelab intends, but to me, code review is a conversation about the changes I'm responsible for making, not a google doc where anyone can change the content I'm about to push.
My point was orthogonal, though. I don't want a reviewer directly making committed changes to my code, period. Giving me control isn't about preventing inadvertent calls to sedy. It's about handling when someone suggests a substitution that I don't agree with, or want to reword.
My mindset might be different from what Marmelab intends, but to me, code review is a conversation about the changes I'm responsible for making, not a google doc where anyone can change the content I'm about to push.
I take note of your feedback, thanks!
If you only support substitutions (the "s" command) maybe @sedy <separator><regexp><separator><replacement><separator><flags> that way you could e.g.
@sedy |foo/bar|foo|g
?Thank you very much for your feedback.
For third point: it is actually the case! We have not implemented all the features of sed for this reason.
I keep the two first points to my list of requested features, it's very interesting!
For third point: it is actually the case! We have not implemented all the features of sed for this reason.
I keep the two first points to my list of requested features, it's very interesting!
Regarding #1, it should be a simple fix once the Reactions API (https://developer.github.com/v3/reactions/) comes out of the preview state.
I share with fiatjaf's sentiment; this is an amazingly cool piece of hackery (that will probably be very useful to many people), but I won't use it.
For those simple types of changes, I like to amend the original commit, rather than make a new one. Of course, I come from using `git send-email` to send patches to a mailing list, where you are expected to send "[PATCH v2]" after you get feedback.
For those simple types of changes, I like to amend the original commit, rather than make a new one. Of course, I come from using `git send-email` to send patches to a mailing list, where you are expected to send "[PATCH v2]" after you get feedback.
What about a command s/[search]/[replace]/a for "--amend"?
That could win me over! I'm not sure what the appropriate answer is to "who should it put in the committer line?" (especially if author and committer are already different), but that's not a huge problem, I don't think.
It would be a problem for me - suddenly, my local branch and the Github branch are no longer in sync, and I have to untangle this mess before continuing development. (And if I don't notice that Sedy is changing the revision history, and only realize it when I've made a couple of commits and try to push...)
You could do an interactive rebase and make all the Sedy commits fixups.
I like the idea.
Using `s/foo/bar/f` for a `git commit --fixup` of the last commit who modify this change (only if part of the current PR)
Using `s/foo/bar/f` for a `git commit --fixup` of the last commit who modify this change (only if part of the current PR)
"Linux sed"? There's no mention of Linux at https://en.wikipedia.org/wiki/Sed
I forget the exact circumstances, but I was once in a W3C IRC chatroom which had a bot that produced a transcript and obeyed sed commands. It's pretty amazing to be able to type s/their/they're/ and have it actually take effect.
Skype chat does this currently, but only for the last comment, I think.
Whoa.
I will not use this, but I can say this is an amazing piece of hackership.
I will not use this, but I can say this is an amazing piece of hackership.
In the first screenshot, how does the bot know which instance of 'products' to change? Also /s/themself/themselves for the second one although chances are Sedy can't edit screenshots.
The default rule for now is "change all instance of that word in the whole line".
We'll fine-tune that behavior with the feedback we'll have.
We'll fine-tune that behavior with the feedback we'll have.
i suggest matching the behavior of slack/skype/hipchat: change only the first instance except in the case of /g (in which case, maybe any instance in the hunk?).
I was kind of hoping for 'we're just trying to make fzaninotto look bad'.
you're killing me
What I do to point out differences is to provide a comment with a code block with diff format.
```diff
-- old text
++ new text
```
Github will highlight that as a diff.
```diff
-- old text
++ new text
```
Github will highlight that as a diff.
Didn't know that, that's really nice.
I know in bitbucket and others have a nice feature that allows you to edit code inline on the web browser and commit the changes - wouldn't that be a safer approach than editing via commenting? I don't think it would take much more time either.
Maybe doing the reverse, if a change is made through inline edit automatically add a comment with the specific change made.
I just can foresee undesired consequences. Someone not escaping their sed correctly (I.e. they try to replace a string with a slash or apostrophe)
Maybe doing the reverse, if a change is made through inline edit automatically add a comment with the specific change made.
I just can foresee undesired consequences. Someone not escaping their sed correctly (I.e. they try to replace a string with a slash or apostrophe)
GitHub has that feature too.
But it's a lot of context switching to edit the file, commit and return to your code review where you'll need to build the code architecture in your head again.
All of that is explained on the article.
All of that is explained on the article.
`/s/foo/bar` syntax works in Slack as well!
How much of the power of sed does it bring?
I feel like other sed expressions might be even more useful in this format. For example:
I feel like other sed expressions might be even more useful in this format. For example:
200i #TODO optimize this
s/.*goto.*/cowsay \0/e
T
s%^%//%
s%\n%\0//%g
Does s:/bin:/usr/bin:g work?
The global flag is planned to edit an entire file. And be able to change the separator could be a very nice feature! Thanks for the idea.
The project is still young and we'll implement in priority what save time to developers.
The project is still young and we'll implement in priority what save time to developers.
Three features I'd want before using it:
1) Rather than triggering Sedy immediately on a reviewer comment, I'd like the trigger to be the original requester reacting to the comment with a thumbs-up. The requester knows what they're trying to say, and they should decide if the changes get made.
2) I wish there were an option to restrict it to comments for supported languages. Your examples are just changing markdown (not code), and I think rightfully so — I can easily see this tool becoming a way for a senior dev reviewer to attempt to avoid the back-and-forth with a junior dev by just posting some complex code substitutions... substitutions which could easily screw things up.
3) sed replacement actually seems too powerful for this job. For instance, if I want to make a replacement like:
...I really don't want to try escaping that without feedback. I'd rather have direct string replacement without regex than the full power of sed. This probably goes against your design goals, but for me it would be much more useful.