Support for accepting / rejecting refactorings via GitHub UI about sourcery HOT 4 OPEN

Sourcery refactors at the function level at the moment but at some point it will be extended to refactor at the class and module level. It would be fairly easy to restrict it currently to only refactor the functions that have been changed but even then it may make a change that is not visible inside the diff - like reusing a new local variable or deleting a assignment that is no longer used. This means that the GitHub comment suggestion approach is too restrictive for us to use.

Repeatedly suggesting a refactoring that you don't want would definitely be annoying. We will be introducing configuration soon that will allow you to disable specific refactorings, either at the function level via a comment, or globally in the configuration file. Obviously the question then becomes how do I add these without having to manually add a new commit to the Sourcery PR. I think this is where the emoji approach could work well, if it were used to update the PR by adding these comments (with a 👎 ) or global configuration (😕 ).

from sourcery.

PRs should have comments for each refactoring, I introduced a bug today which stopped them appearing for a while but that has now been resolved.

Your idea for using GitHub comment suggestions is an excellent one and one we have considered in the past. Unfortunately the current GitHub UI limits the placement of these comments to within the existing PR diff - https://developer.github.com/v3/pulls/comments/#create-a-comment

Note: The position value equals the number of lines down from the first "@@" hunk header in the file you want to add a comment. The line just below the "@@" line is position 1, the next line is position 2, and so on. The position in the diff continues to increase through lines of whitespace and additional hunks until the beginning of a new file.

That means if a Sourcery refactoring changes lines outside of the changed lines in the PR it can't be suggested. This can happen quite frequently and so we decided to create an extra PR on top.

I agree that it would be a much better workflow to just choose which ones are wanted in GitHub. Maybe there each refactoring could be rejected or permanently turned off by replying with a specific comment, or adding a reaction like 👎 or 😕 , and then Sourcery would update the PR appropriately.

Would that address this issue?

from sourcery.

That means if a Sourcery refactoring changes lines outside of the changed lines in the PR it can't be suggested.

I was under the impression it only refactors the current diff unless you specifically ask it to refactor a branch. Could you elaborate your intentions to always refactor the complete file?

Lets say Sourcery suggests a refactoring I do not want. As is it will suggest this over and over again if a modify the file without touching the "problematic" code.

I agree that it would be a much better workflow to just choose which ones are wanted in GitHub. Maybe there each refactoring could be rejected or permanently turned off by replying with a specific comment, or adding a reaction like 👎 or 😕 , and then Sourcery would update the PR appropriately.
Would that address this issue?

It would definitely ease the workflow. Personally, I like the emoji reaction better. Depending on your answer to my question above I would still opt for the review / suggestion approach.

from sourcery.

It would be fairly easy to restrict it currently to only refactor the functions that have been changed but even then it may make a change that is not visible inside the diff - like reusing a new local variable or deleting a assignment that is no longer used.

Makes sense, I didn't think of that. Thanks for the explanation.

from sourcery.