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

feat: DoH ability to use custom servers #256

Closed
wants to merge 1 commit into from

Conversation

M0NsTeRRR
Copy link
Contributor

Related Issues

Fixes : #254

@M0NsTeRRR
Copy link
Contributor Author

I've removed the quick fix for #253, tests are expected to fail until it's rebased with hyperglass 2.0.2

@M0NsTeRRR
Copy link
Contributor Author

M0NsTeRRR commented Jun 1, 2024

@thatmattlove as an open-source contributor, I wanted to express my concern about how this PR was merged. The minimum courtesy when someone contributes is to keep the author credited as a thank you for their contribution. On GitHub, you have the right to edit my PR by default. If you need to add something, feel free to do so and then merge it. Of course, you have the final word as you are the maintainer.

I think (and hope) that you did this for convenience, and I trust that next time it will be merged properly.

PS : As an user, thanks you for your project ;)

@thatmattlove
Copy link
Owner

Please refer to the Contributing Policy, namely:

Because I've been solo-maintaining and building hyperglass since around April 2019, I've become pretty particular about things that might seem trivial to someone just trying to help out. While I absolutely welcome development contributions, please don't be offended if pull requests are denied, or if I request things to be done a certain way.

I appreciate your willingness to contribute your time and effort to hyperglass, however, there were several reasons your PR was not merged:

  1. I was already working on a solution after I saw feature: ability to use custom DoH server #254, and didn't see your PR until those efforts were underway.
  2. Your PR introduced a breaking change by removing the name field.
  3. CI tests for your PR failed, though I have not looked into why.

In the future, if you plan to implement a fix for an issue you've raised, please indicate that intent in the issue so I know not to start working on it.

@M0NsTeRRR
Copy link
Contributor Author

Because I've been solo-maintaining and building hyperglass since around April 2019, I've become pretty particular about things that might seem trivial to someone just trying to help out. While I absolutely welcome development contributions, please don't be offended if pull requests are denied, or if I request things to be done a certain way.

Sure, I understand, and I've read it, but you should not say contributions are welcome when you say right after that you can deny them just like that. You can deny it as you are the maintainer if we can't agree on how it must be done, and that's okay, but you can just request changes when someone has started working on it.

I was already working on a solution after I saw #254, and didn't see your PR until those efforts were underway.

No worries, it happens, but my PR was linked to the issue.

Your PR introduced a breaking change by removing the name field.

Yes, you are right.

CI tests for your PR failed, though I have not looked into why.

I've written in the next comment why; it's due to the "." bug. I've removed the temporary fix to get it merged easily. Rebasing on it is the only thing that was needed.

@M0NsTeRRR M0NsTeRRR deleted the feat/doh branch June 1, 2024 22:02
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.

feature: ability to use custom DoH server
2 participants