-
Notifications
You must be signed in to change notification settings - Fork 34
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
Essentially Rewrite HellPot + Fun Bonus Features #162
Conversation
…zche While I do think all bots enjoy Nietzche (who doesn't?), I think we should take a stance to educate them. What better way than to be able to choose from any book! Personal suggestions include: - The Sorrows of Young Werther by Goethe - Any political manifesto - The Declaration of Independence etc. etc.
This removes globals from `heffalumpt/`, which are hard to reason about, easy to get wrong, and should not be created if they are never used. A possible drawbacks is if you would create multiple new defaults, but this should never be the case.
This prints the short variants (like `-c` for `--config`) in the help. Also fixes bug where only `-h` flag works, not `--help`.
…n (missed a spot)
@ShadowJonathan your commits are not signed, you'll probably need to rebase.. I will likely just make a separate PR until we get that sorted. |
I'm not sure what you mean, do you want to exclude my commits, or...? #137 is already merged, I'm unsure what you mean. I haven't started signing my commits due to the hassle that I've experienced with it in the past, apologies that they're not indeed signed, though I did not know of this required when I was making #137. |
it's merged to dev, we are trying to merge dev to main |
additionally: - fix `-c` flag - make main package test better
@yunginnanet if signed commits are required, you should require them for all commits, on all branches and for all PRs. You can't expect someone to fix it after the fact. You can fix it by doing an interactive rebase, and changing the committer (not the author!) to be yourself and doing a sign-and-amend. |
And yes, that will require a force push. But that's less hassle than excluding those commits and requesting a PR with signed commits be made. |
While I agree that the inconsistency is annoying, branch protections on main vs branch protection on dev is using the system exactly as intended. Additionally, I have had to fix my commits for many many PRs I've made to other projects and do force pushes myself.. For example, my PR/CL for https://github.com/golang/go 1 I had to rebase against master several times and force push to my branch. Footnotes
|
That said, since this has already been merged I am willing to put in the work on my end to help adjust the development branch once they make a new branch and do a rebase. I can't sign their commits for them... I am not them. If I hack apart the history in the way you describe and do a force push, I have now obliterated accountability for the changes made by the author. If I merge without the signing requirements being met, I have now obliterated accountability for the changes made by the author. |
You can sign my commits, or remove them, at this point I don't care. This should have been brought up before you merged the PR, and not now. I might've been partial to signing my commits in some way (by pushing them through the web interface), but now that has been more than 2 months ago, I assumed all development was done when you accepted the merge, and now I'm not interested in a retroactive problem, that I wasn't made aware of in any way, that should've been brought up in that PR in the first place. Sorry. |
That's perfectly fine. |
Superseded by #163 |
Well, it broke down here. Also, yes, you can sign commits authored by others. Simply become the committer (≠ author!), and sign the commit. Here's an example for me doing precisely that on another project, with a cherry-picked commit: CatCatNya/catstodon@8e8cf36 |
Yes, which means I then adopt accountability for the commits. HellPot is a honeypot, therefore it is security minded software. I need collaborators that have contributions that make it to the main branch to be prepared to take full responsibility for their contributions. If they are not, that is fine, but their code likely will need to stay in a development branch. It is notable that the contributions in question modified the core heffalump package. This package, while still relatively minimal risk thanks to Go's memory safety, is by far the most sensitive and critical part of the entire project. I am okay with this outcome. |
@yunginnanet Sure, but how do you ever expect a commit that is unsigned due to zero signage requirements on the dev branch to enter the main branch? That just discourages contributions. |
I am also fine with making the requirements consistent, this was certainly not an intentional idiosyncrasy. |
Overview
I accidentally a whole
coca cola bottleHellPotThis really is essentially a rewrite.
The only thing that remains untouched more or less is the heffalump core.
This should resolve countless idiosyncrasies in configuration behavior.
Additionally, there are now unit tests.
New Features
strings.Contains
match functionalityNew features by
are scheduled to hitch a ride, pending further review and testing.
Slimfast
Further slimming of dependencies
Issues resolved