Skip to content
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

[RFC] Support repository-specific configuration #110

Closed
chshersh opened this issue Oct 18, 2019 · 10 comments
Closed

[RFC] Support repository-specific configuration #110

chshersh opened this issue Oct 18, 2019 · 10 comments

Comments

@chshersh
Copy link
Contributor

It was decided that's it's a good idea to introduce a file like hintman.toml or .hintman.toml that users can put in their repos and modify to adjust settings. This issue is a discussion on possible features of this config.

I'm just going to dump a bunch of ideas. So feel free to comment on what you find useful:

In addition to supported features it would be nice to have:

  • Some default TOML file with comments that users can download and modify locally without need to create it from scratch.
@sshine
Copy link

sshine commented Oct 21, 2019

  • The possibility to change the wording of these comments.
  • Whether Hintman should request changes or remain neutral when there are hints.
  • Whether Hintman should approve or remain neutral when there are no hints.

@vrom911
Copy link
Member

vrom911 commented Oct 21, 2019

The possibility to change the wording of these comments.

I'm sorry, but how this affects anything? We would like to maintain features that only could bring any benefit.

Whether Hintman should request changes or remain neutral when there are hints.
Whether Hintman should approve or remain neutral when there are no hints

If you would use Hitman you would see that it's approval/rejection is not counted as Hintman is just a bot not a collaborator/member. So there is no benefit from that as well.

@sshine
Copy link

sshine commented Oct 21, 2019

The possibility to change the wording of these comments.

how this affects anything?

Since you phrase your rejection of my proposals as a question, I'll answer:

Because it's not immediately obvious what's going on here:

drunk-bot

Is it drunk, or stuck in a fortune cookie?

I might like for that comment to say "Passed automated linting." or nothing at all.

Whether Hintman should request changes or remain neutral when there are hints.
Whether Hintman should approve or remain neutral when there are no hints

it's approval/rejection is not counted as Hintman is just a bot not a collaborator/member

I'm not sure if that's immediately obvious to everyone; the only GitHub app I've used so far is "wip".

This fact is emphasised with the "✓" being gray rather than green.

To those who are uninitiated to GitHub's color codes (or simply red-green color blind), it may falsely indicate that a PR was approved when, in fact, it just passed automated linting. Such misunderstandings can be mitigated, in part by not "approving", which ambiguously means either "the linter found no errors" or "the maintainer thinks this should be merged", and in part by making it obvious in text exactly what happened: The PR passed automated linting.

These are suggestions; if you have chosen the truths that you like, it is after all open source.

@vrom911
Copy link
Member

vrom911 commented Oct 21, 2019

I don't think your tone is appropriate for the open source. We are spending our spare time on the project and I think none of us deserves such attacks.

If you had strong opinions about your decision you could explicitly write that, so we could see that you propose something with the particular reason and help you with the understanding of what is going on, but instead, you decided to demand something and use inappropriate words like "drunk" and similar.

I would need to ask you to stop behaving this way in our organization. We have the Code of Conduct and we would like everybody who wants to take part in the Kowainik projects to familiarise oneself with it. Our goal is that everybody feels secure in here, but your tone doesn't imply it.

@sshine
Copy link

sshine commented Oct 21, 2019

I'm sorry for mocking the humor embedded in the bot's response, and I'm sorry for using this as an attack on why you may not want to make its responses optional. I've not demanded anything but the assumption that my suggestion is not ridiculous by default.

I'd like to enable the bot for a repo that sees a lot of PRs from people who have never contributed to open source before. I suspect that the humor / philosophy embedded will leave confusion as to what's going on, since this would be the first (albeit automated) voice of that project to a new contributor.

I understand if you'd like to keep configuration low, as a general matter of preference.

I've described how the two types of configuration I suggested can be useful in one instance.

@popzxc
Copy link
Collaborator

popzxc commented Dec 14, 2019

Despite the tone of the suggestion, I agree with it: I'd like to enable hintman on several repositories I'm participate in as well, but currently a bit afraid -- some people may not understand the irony.

IMHO, the possibility to switch the bot phrasing to something formal/neutral is a good feature which can lead to a wider audience approval.

@popzxc
Copy link
Collaborator

popzxc commented Dec 14, 2019

Other topic for clarification is behaviour of the hintman if it fails to parse repo configuration.
Should it fallback to the default configuration, report the unability to parse configuration to one/every PR, or do something else?

@vrom911
Copy link
Member

vrom911 commented Dec 18, 2019

@popzxc, thanks for your input! As I mention I do understand the necessity of such a request, it completely has a point.
Don't mind the previous messages, they are something that is not usually happening in the organization, but we try to cut off toxic and offensive conversations.

Other topic for clarification is behaviour of the hintman if it fails to parse repo configuration.

Yes, this is an important question, thanks for bringing that up. I would say that the default configuration with the top PR comment saying about the problem and that it's switched to the default config makes sense to me. I guess it's the best way to trace such issues at the moment.

@popzxc
Copy link
Collaborator

popzxc commented Jan 21, 2020

I'm still thinking about this issue, and got an idea on behaviour of hintman if it fails to read the repo configuration.

The solution is really simple: hintman can just create a GitHub issue in the repo 🙂
If config cannot be parsed, hintman creates an issue describing the error and telling that it falls back to the default configuration.

This will require hintman to monitor the state of .hintman.toml after creating the issue (if it was updated, the issue should either be closed or updated with a comment that config is still incorrect), but I think that this approach is correct logically.

What do you think?

@chshersh
Copy link
Contributor Author

@popzxc This is a nice proposal and a possible direction for improvements in this area 🙂 However, this will introduce new complications and involve some discussions around design. I propose for now (well, once the configuration is implemented) to simply print error message text inside review comment and fall back to default configuration. And discuss submitting of errors and news via issues under a separate issue.

@vrom911 vrom911 closed this as completed Jun 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants