Skip to content

Update user.php#12843

Merged
danielkerr merged 1 commit intoopencart:masterfrom
TheCartpenter:patch-1346
Oct 29, 2023
Merged

Update user.php#12843
danielkerr merged 1 commit intoopencart:masterfrom
TheCartpenter:patch-1346

Conversation

@TheCartpenter
Copy link
Copy Markdown
Contributor

No description provided.

@ppalashturov
Copy link
Copy Markdown
Contributor

Stop spamming please make a PR with all the commits not file by file!

@TheCartpenter
Copy link
Copy Markdown
Contributor Author

Stop spamming please make a PR with all the commits not file by file!

Another late in the news. That's fine. All you have to do is to read this commit post before posting against the expectations: #12616 (comment)

@TheCartpenter
Copy link
Copy Markdown
Contributor Author

Besides, there are also commits containing multiple changes rather than posting each changes for every commit. Otherwise, it would create conflicts on anyhow.

@batumibiz
Copy link
Copy Markdown
Contributor

@danielkerr please stop this spam crap from TheCarpenter.
He doesn't want to understand that all these small changes can be made in one PR, and creates a serious inconvenience for everyone else.

Please figure it out, it’s impossible to tolerate this anymore.

@TheCartpenter
Copy link
Copy Markdown
Contributor Author

@danielkerr please stop this spam crap from TheCarpenter. He doesn't want to understand that all these small changes can be made in one PR, and creates a serious inconvenience for everyone else.

Please figure it out, it’s impossible to tolerate this anymore.

Email notifications are based on personal preferences. You don't like receiving my commit post as notifications while other users are also doing it on this repository? Then, put it on the ignore list!

@mhcwebdesign
Copy link
Copy Markdown
Contributor

I counted 58 pull requests from TheCartpenter during the past 24 hours, mostly cosmetic changes. So yes, it would have made sense to combine them into a single one. The next release will have an unnecessary long change history.

@TheCartpenter
Copy link
Copy Markdown
Contributor Author

I counted 58 pull requests from TheCartpenter during the past 24 hours, mostly cosmetic changes. So yes, it would have made sense to combine them into a single one. The next release will have an unnecessary long change history.

Cosmetic changes are only a matter of opinion. The proposed changes are already located elsewhere in the core as I am only filling the rest of the blanks. If you believe these changes will be unnecessary to be announced in the changelog, then it means you do not fully understand the changes that's been already made in the core.

@condor2
Copy link
Copy Markdown
Contributor

condor2 commented Oct 29, 2023

Who don't like the 'SPAM'....just change notifcation settings.....

@batumibiz
Copy link
Copy Markdown
Contributor

Who don't like the 'SPAM'....just change notifcation settings.....

This creates some inconvenience.
A more correct way is to fight the source of spam. This should not happen on a serious project.

@condor2
Copy link
Copy Markdown
Contributor

condor2 commented Oct 29, 2023

This creates some inconvenience. .

For what you need Email notification on other project?

@batumibiz
Copy link
Copy Markdown
Contributor

For what you need Email notification on other project?

What are you talking about now?
The main complaint here is not the large number of notifications, but the fact that the GIT history is clogged. 80% of minor changes that do not correct anything and do not affect the algorithm can be combined into 1 commit and, accordingly, 1 PR can be sent.

@condor2
Copy link
Copy Markdown
Contributor

condor2 commented Oct 29, 2023

The main complaint here is not the large number of notifications

And what is the problem?....

Just change Github settings so that notifications to be only on github page and not e-mail.....

@mhcwebdesign
Copy link
Copy Markdown
Contributor

The main complaint here is not the large number of notifications

And what is the problem?....

Just change Github settings so that notifications to be only on github page and not e-mail.....

Then how do you prevent the clogged GIT history? And how can we selectively prevent notifications coming from TheCartpenter, but not others?

@stalker780
Copy link
Copy Markdown
Contributor

@mhcwebdesign I have added a filter in gmail which deletes every message containing sender TheCartpenter couple years ago.

It's vain to ask him to change his behavior. He does not understand what he is doing.
He simply shows his disrespect to OC community and breaking github notifications in public repo.
Perhaps he behaves the same in the street and other public places :)

Try to contact github support if Daniel ignores this. But I don't think this will help a lot.

@TheCartpenter
Copy link
Copy Markdown
Contributor Author

@mhcwebdesign I have added a filter in gmail which deletes every message containing sender TheCartpenter couple years ago.

It's vain to ask him to change his behavior. He does not understand what he is doing. He simply shows his disrespect to OC community and breaking github notifications in public repo. Perhaps he behaves the same in the street and other public places :)

Try to contact github support if Daniel ignores this. But I don't think this will help a lot.

No disrespect has been shown on the commit, as I am still following Github's regulations as to post suggested code changes. No official regulations dictates that these posts needs to be put into a single commit altogether. Since these arguments seem to be about email notifications, as said on my previous post, email notifications are based on personal preferences. What users decides with their notifications are decisions based on their own.

@danielkerr danielkerr merged commit 80ed702 into opencart:master Oct 29, 2023
@danielkerr
Copy link
Copy Markdown
Member

yeah leave him alone. i appreciate the help. i don't include all his commits but he does have some good push requests! i need all the help i can get. i prefer people people helping than complaining.

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.

7 participants