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

fix reversion #923

Merged
merged 1 commit into from
Nov 7, 2024
Merged

Conversation

nnyyxxxx
Copy link
Contributor

@nnyyxxxx nnyyxxxx commented Nov 6, 2024

Type of Change

  • Hotfix

Description

@ChrisTitusTech this pr fixes a reversion caused by #755

this pr reimplements the functionality added into #729 that stopped working due to #755

Copy link
Contributor

@lj3954 lj3954 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please correct the title; commit log will be more readable if you specify what regression is fixed, and if it doesn't identify a regression as a reversion.

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

why does it matter?

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

the pull request title and commit title are both self explanatory and more information can be given in the diff

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

i don't understand what the point of changing the titles are

@lj3954
Copy link
Contributor

lj3954 commented Nov 7, 2024

Commit log is useful to find when certain changes were made, to identify regressions or otherwise. Commit messages should therefore be descriptive, and identify generally what changes have been made.
Since Chris uses squash and merge for most of these PRs, the title is the most important thing to get right.

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

all merged pull requests have a PR number in the title, all information regarding the commit is in this PRs description. What you're suggesting is unnecessary because of this.

@lj3954
Copy link
Contributor

lj3954 commented Nov 7, 2024

That is utter nonsense. Telling anyone reading the commit log to visit each merge request to see anything about what the commit is intended to do is ridiculous, that's the purpose of the commit's title.
This pattern with you isn't good to see. Anyone suggesting improvements of any sort is immediately met with a nonsensical debate about why we would even need to do x or y.

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

the pull request template provides a description header for a reason.

@lj3954
Copy link
Contributor

lj3954 commented Nov 7, 2024

A description is where you elaborate on why the changes you made are beneficial to the project, why you did certain things in the code that may be harder to understand or otherwise may be questioned, a granular list of specific changes, etc. Not to express the most basic of information. That's what the PR's title, and specific commit names (which aren't particularly relevant when squashing), are intended for. These features don't exist for no reason. If every PR or commit was titled "change a few things" or "fix a bug", nobody would be able to get anything done.

A proper title for your PR would be something along the lines of fix: Correct regression in scrolling behaviour (#755)

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

#923 (comment)

@lj3954

This comment was marked as off-topic.

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

it's a simple one line fix, no need to sit here and argue with me about something as stupid as a PR title / commit title for one hour straight.

@lj3954
Copy link
Contributor

lj3954 commented Nov 7, 2024

Indeed, it's not worth arguing over a 1 line fix. That's why I find it strange that you decided to start an argument when I suggested such a change.

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

the only one who started an argument here was you.

#923 (review)

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

why say this specifically in my pull request? why not go through all of the closed ones and do it there as well? I'm sure a bunch of merged pull requests have titles just like this one in the log.

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

anyways you shouldn't be barking orders at me like you have been for months now, i'm pretty sure you aren't a part of the maintainers in this repo. If Chris wants me to change the commit name and pr title then so be it, same goes for the rest of the maintainers.

@lj3954
Copy link
Contributor

lj3954 commented Nov 7, 2024

Suggesting a change is not starting an argument. You've suggested dozens of changes on PRs, are you trying to argue with everyone? If you view it that way, you should heavily reconsider what the purpose of code review is.

I can't find many PRs with such non-descriptive titles, especially from a major contributor to the project with a very easy to document change. Requesting a small change now would be likely to increase the chance of you and others reading this to improve the documentation of their changes in the future, which increases maintainability of this repository as well as other codebases.

"Barking orders" is a strange way of framing a simple and reasonable suggestion, especially coming from someone who's "requested changes" of "close this PR immediately" with a list of grievances, several times, to a number of different contributors.

Please reference the Code of Conduct of this repository to determine what appropriate behaviour does and doesn't look like.

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

Suggesting a change is not starting an argument. You've suggested dozens of changes on PRs, are you trying to argue with everyone? If you view it that way, you should heavily reconsider what the purpose of code review is.

yet the change you're suggesting is unrelated to code review at all.

"Barking orders" is a strange way of framing a simple and reasonable suggestion, especially coming from someone who's "requested changes" of "close this PR immediately" with a list of grievances, several times, to a number of different contributors.

All PRs i have done that in were rubbish and I had several reasons to do so.

I can't find many PRs with such non-descriptive titles, especially from a major contributor to the project with a very easy to document change. Requesting a small change now would be likely to increase the chance of you and others reading this to improve the documentation of their changes in the future, which increases maintainability of this repository as well as other codebases.

I highly doubt that you can't. Try looking one more time.

Please reference the Code of Conduct of this repository to determine what appropriate behaviour does and doesn't look like.

I have already read through it and none of my behaviour is breaking it. Why even bring it up?

@lj3954
Copy link
Contributor

lj3954 commented Nov 7, 2024

Please read up as to what code review is. Commits certainly are relevant to it, and suggestions of simple improvements are almost certainly bound to help you in whatever comes your way in the future.

Dismissing every PR you have a criticism of as "rubbish" is not acceptable, and you should once again reference the code of conduct. I can name several cases where you've done just this for perfectly valid PRs, with the most clear example being found in #751. (To be clear, a contributor who submits an entirely LLM generated PR with code that won't even compile shouldn't be attacked in such a way either). The same issue about your behaviour was raised there by another contributor, referencing several examples. You must reflect upon this behaviour, it will without a doubt negatively impact your professional life if you cannot work it out.

The only commit I can see within the last few pages of the commit log with a non-descriptive title is from #889, but documenting which changes were made there wouldn't be particularly easy, and virtually none of the other commits I've seen from this contributor share such an issue.

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

well, the commit title and pr title is going to stay the same regardless of your opinion. I'm not going to sit here and argue with you over something this petty.

@lj3954
Copy link
Contributor

lj3954 commented Nov 7, 2024

I think @ChrisTitusTech should comment on this pattern of inappropriate behaviour. This is nowhere near isolated, and has been going on for several months, directed at several contributors. It belongs absolutely nowhere near any open source project (or frankly, anywhere), and needs to be worked out.

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

please point out where i was "inappropriate" in this pull request, i would like to see.

@lj3954
Copy link
Contributor

lj3954 commented Nov 7, 2024

Provoking unreasonable & pointless arguments after being suggested changes is not appropriate behaviour, the code of conduct very clearly states this (Giving and gracefully accepting constructive feedback). This is nowhere near isolated though, as I mentioned, and I've seen far worse from you several times, which is why I'd like Chris to comment on it.

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

Provoking arguments after being suggested changes is not appropriate behaviour, the code of conduct very clearly states this (Giving and gracefully accepting constructive feedback). This is nowhere near isolated though, as I mentioned, and I've seen far worse from you several times, which is why I'd like Chris to comment on it.

again the only one who provoked an argument here was you by suggesting an unnecessary change that wasnt even related to the changes in this pull request.

#923 (review)

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

this is not your pull request, i clearly should not have to follow the standards you're trying to push here.

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

there's a reason why it says "suggested changes"

@lj3954
Copy link
Contributor

lj3954 commented Nov 7, 2024

Again, suggesting changes is not arguing. Leaving 3 comments, all saying it's pointless to change anything, with an aggressive tone, and then resorting to personal attacks absolutely is. And once again, this behaviour is nowhere near isolated. You've called 2 contributors "disgusting" for creating PRs with changes you didn't like, called PRs you had criticism of "rubbish" and demanded their closure, often with unsubstantiated claims, and generally worked to reduce the amount of productive discussion within this repository.

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

Again, suggesting changes is not arguing. Leaving 3 comments, all saying it's pointless to change anything, with an aggressive tone, and then resorting to personal attacks absolutely is. And once again, this behaviour is nowhere near isolated. You've called 2 contributors "disgusting" for creating PRs with changes you didn't like, called PRs you had criticism of "rubbish" and demanded their closure, often with unsubstantiated claims, and generally worked to reduce the amount of productive discussion within this repository.

again please point out where the "personal attacks" are in this pull request, i would like to see what you're referring to.

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

also if you want to point out things like "disgusting", etc. Then tell Chris that as well, he has done the same exact thing earlier this day.

ChrisTitusTech/winutil#3005 (comment)
image

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

the standards you're trying to push out here is absolutely crazy, am I not allowed to sit here and debate with you over why I should do x, y, and z? Or will you call everything i said in that debate "personal attacks" as well?

@lj3954
Copy link
Contributor

lj3954 commented Nov 7, 2024

#923 (comment)
This comment is effectively nothing but an attack upon my character.

If you have a different and reasonable opinion on how code should be written, or anything of the like, sure, don't change anything about it; wait for the maintainers. Several comments you've left show that you certainly don't appear to, though, and that you're just leaving the title to stand your ground in an argument, which isn't a particularly good look.

It's not just me saying this about your behaviour, other contributors have raised exactly the same and no changes have been made. (#751, again, a prime example). Pointing to a review I've made on your PR, with nothing but constructive feedback on how it could be improved, and acting like it was intended to spark a debate which devolves into attacks & condescending remarks is just not good character.

The comment you showed from Chris was almost certainly made in jest. The difference is rather clear, compared to the comments you've made calling contributors "disgusting" and making demands (also note the phrasing of my review here, compared to your review on #751 and several other PRs you've had disagreements with).

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

#923 (comment) This comment is effectively nothing but an attack upon my character.

If you have a different and reasonable opinion on how code should be written, or anything of the like, sure, don't change anything about it; wait for the maintainers. Several comments you've left show that you certainly don't appear to, though, and that you're just leaving the title to stand your ground in an argument, which isn't a particularly good look.

It's not just me saying this about your behaviour, other contributors have raised exactly the same and no changes have been made. (#751, again, a prime example). Pointing to a review I've made on your PR, with nothing but constructive feedback on how it could be improved, and acting like it was intended to spark a debate which devolves into attacks & condescending remarks is just not good character.

The comment you showed from Chris was almost certainly made in jest. The difference is rather clear, compared to the comments you've made calling contributors "disgusting" and making demands (also note the phrasing of my review here, compared to your review on #751 and several other PRs you've had disagreements with).

again never once have i called any contributors "disgusting" always have i referred to the pr in question and the changes being made in question, secondly the comment you linked is not a personal attack in the slightest @adamperkowski Adam a maintainer of this project can confirm if he wishes to comment on this matter.

With this attitude its clear to me that you don't like being debated upon your opinions. Like I said earlier the PR title and commit message is not getting changed whether you like it or not, so just drop the argument, It's completely pointless.

Every comment made by me is logged in github if anyone wishes they can go back and look through my messages to see if what i am saying here is true (it most certainly is)

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

every review i have made is towards the changes inside of the pull requests not something as petty as a pr title or commit message.

This is just outrageous behaviour from you, honestly. Starting an argument with me over a commit message and PR title.

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

earlier in this pull request i shared exactly why i didn't want to apply your suggested changes, yet you stay here and argue with me and bring up nonsense that is completely unrelated to this pull request. Shameful.

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

and I'd like to quote what i said earlier:

"This is not your pull request".

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

I'm ending this debate here, I'm not going to feed into the trolling any longer.

If you want you can stay here and hallucinate more things into existence, that's fine.

@lj3954
Copy link
Contributor

lj3954 commented Nov 7, 2024

Adam has generally stood aside whilst you've engaged in attacks upon contributors, and I think that's probably the main reason why he has the ability to moderate discussions here, while you don't.

Let's take a look at how far you're trying to stretch the truth for most of these.

again never once have i called any contributors "disgusting"

'i have gave you a solution and you have declined it (for reasons unknown) this is just disgusting.' (#751 (comment))
What's disgusting? The changes? Keep in mind your "suggested solution" was "I can't reproduce this, so there's no issue; close the PR" after he gave you clear instructions on how to reproduce the issue and we ended up resolving the issue with a solution I proposed that you implemented.

'This PR is disgusting, this guy has spent a total of 5+ hours arguing with us over unneeded functionality that hes adding.' (#401 (comment))
Again, dancing around the issue by saying "the PR is disgusting" does not make it any more acceptable. You're very clearly, once again, attacking the contributor who made the PR. A set of changes to the code being 'disgusting' of course does not make much sense.

If you cannot completely rescind these two statements, which are clear examples of inappropriate behaviour, violating the code of conduct, I don't think you should have a place in any open source project. These actions drive away contributors, reduce the amount of productive discussion occurring, and of course are totally unprofessional.

Right now, we are not talking about the title of this PR, to be clear. That was discussed above, and I again don't believe that you seriously have any disagreement with my position (you've provided only vague 'it's not completely necessary' retorts, which is what you've done every time you've wanted to start a non substantive argument for no reason whatsoever). We're discussing your current & past behaviour, which in my eyes (and that of at least one other contributor), is rather toxic. This same discussion has occurred several times, between you and several other contributors, with no changes in your behaviour, which is why I'd like Chris to provide comment.

I'd like to direct you to virtually any of my PRs where other contributors suggested changes, or to PRs from several other contributors, such as #907, where we got a few improvements made (despite the contributor, like you, believing that the changes we wanted weren't that important), and check out how the discussion goes there. We have a productive discussion on the merits of the change, and settle it rather quickly. I've had cases where I've disagreed with someone, and even had rather long discussions about it (see earlier PRs where me and @JustLinuxUser debated the merits of two methods to handle script sourcing, and we settled on his approach), but they've always maintained civility, very much unlike what we see here. Who's the common denominator with all of these examples where unproductive debates & personal attacks occur? It can't be everyone else to blame.

I expect you to read none of this and respond with something you've already said and I've already commented on, so I'm leaving it here. I hope improvements can be made so that we can have productive conversations about code, rather than unprofessional debates focused on topics of little relevance like this one. It feels like I've written virtually exactly this same comment 3 times now, which is a shame; I hope I don't need to do so again. @adamperkowski, I'd recommend locking the thread since discussion about the actual PR in question ended roughly 20 comments ago (#923 (comment)).

@lj3954
Copy link
Contributor

lj3954 commented Nov 7, 2024

And I'd like to, as I've done before in such heated arguments, apologize for the cases where I think I overstepped.
This comment is phrased in a way that appears to attack your abilities ('you'll learn in the future') #923 (comment)
I've retracted this comment, and would retract any others which share such a sentiment if they were brought to my attention. I hope to show an example of what a civil conversation looks like, rather than falling into such inappropriate behaviours that have a negative effect on everyone present.

@lj3954
Copy link
Contributor

lj3954 commented Nov 7, 2024

An to be clear here, for the contents of the PR, I approve of them, other than the title which I of course still think could & should be more detailed. So this is good to merge.

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

if anyone is making personal attacks here it is you.

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

@ChrisTitusTech I'd recommend reading through every comment this guy has made on this PR, he has personally attacked me in his comments several times, even spreading slander as well.

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

not to mention he sparked the argument in the first place... #923 (review)

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

another comment i'd like to reference as well, i'd say it can be considered the start of the argument #923 (comment)

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

This guy has argued with me in the past over nonsense as well, (even doing outrageous things like taking my WHOLE implementation, making a pr with it, and then proceeding to argue with me for over 16 hours straight)

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

i'd like to also add that @lj3954 has personally attacked other contributors as well not just me, i can point out @Real-MullaC especially. I have linked some images below, they should be used as examples of him forcing his opinions onto people.

image
image

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

i'd also like to mention that this guy is being a total hypocrite with him pointing out that i say "please close this pr" when he does it himself as noted from the images above and below:

image

@lj3954
Copy link
Contributor

lj3954 commented Nov 7, 2024

The first comment in the second screenshot you provided indeed shouldn't have been written how it was. I certainly can see that, and I definitely would rather phrase it differently. I would like to still state that what I said was different than your personal attacks on contributors, especially those with completely valid and functional PRs that you just have small disagreements with (as you've done several times), but I definitely think I shouldn't have phrased those comments like I did.

To be clear, the issue is not telling someone to close a PR, but attacking the contributor, or starting arguments about whether or not something is "totally necessary", in some cases claiming a real issue "doesn't exist". I hope you can rescind the dozens of cases, or even the two I very clearly pointed out, where you made very clear attacks on the individuals proposing the changes. It was official project policy (from the roadmap) that GUI work wasn't to start in Q3; the first comment you provided was not problematic whatsoever.

The fact that you're spending so much time trying to accuse me of everything you can, nearly all of which is either just flat out untrue or twisted to the point of unrecognizability (while projecting the same upon me), is totally out of place in an open source project, and clearly violates the code of conduct (as well as GitHub's community guidelines) in a number of ways. With your inability to reflect upon even the worst of what you've said, I truly hope you get sanctioned in some way & are able to reflect upon this in that time. It's for your own sake as well; this sort of behaviour, that appears when you disagree with someone on the most minor of changes, would be nearly certain to negatively impact your professional life as well.

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

you're personally attacking me as well, i am not in violation of any coc as well.

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

not once have i said anything to "personally attack" you yet you keep claiming that i have, same with other contributors it was never directed at them at all.

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

but in those two photos you are clearly attacking the contributor (real-mullac) and then you're saying that i'm making stuff up when the proof is literally right there...

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

the only one slandering here is you.

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

the fact that you have been sitting here attacking me for hours straight makes me question if i want to block you or not so everything argument related just magically ceases to happen

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 7, 2024

Done. I'd like to also ask that this PR gets locked as "Too Heated" -> @adamperkowski please and thanks.

@adamperkowski
Copy link
Collaborator

Well... I think I'm a bit late.

Repository owner locked and limited conversation to collaborators Nov 7, 2024
@ChrisTitusTech
Copy link
Owner

No idea how all these comments spawned from a 2 word PR title and a one line change.

I always welcome feedback but no need to get snarky in the comments and there is enough blame to go around here. Let's not take feedback as personal attacks and also work to make our comments as constructive as possible.

In the end we are here to learn, improve the project and get better. This isn't the first instance of drama between all parties involved and if I have to step in and "fix" this no one will like the result.

Do better.

@ChrisTitusTech ChrisTitusTech merged commit 421044f into ChrisTitusTech:main Nov 7, 2024
2 checks passed
@nnyyxxxx nnyyxxxx deleted the fix_reversion branch November 7, 2024 21:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants