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

refactor: updateActiveDate query builder #520

Merged
merged 1 commit into from
Dec 4, 2022

Conversation

kpeu3u
Copy link
Contributor

@kpeu3u kpeu3u commented Nov 11, 2022

No description provided.

@kenjis
Copy link
Member

kenjis commented Nov 11, 2022

Sorry, what's the point?

Copy link
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

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

The method does a lot more bootstrapping than just accessing the property. It also isn't final so it is possible someone is extending it for additional behavior.

I'm good with this change.

@datamweb datamweb added the GPG-Signing needed Pull requests that need GPG-Signing label Nov 12, 2022
@datamweb
Copy link
Collaborator

Hello,
Your commit is not signed, please check the following to sign it.

See :
Adding A GPG Key To Your Github Account
GPG Signing Old Commits

@kenjis
Copy link
Member

kenjis commented Nov 12, 2022

And improving the commit message is welcome.

Commit messages are expected to be descriptive of why and what you changed specifically. Commit messages like "Fixes #1234" would be asked by the reviewer to be revised.
https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/workflow.md#commit-messages

@datamweb datamweb added the waiting for info Issues or pull requests that need further clarification from the author label Nov 15, 2022
@kpeu3u kpeu3u force-pushed the fix-set-last-active branch 3 times, most recently from f188c37 to 3e49082 Compare December 1, 2022 08:26
@datamweb datamweb added enhancement New feature or request and removed waiting for info Issues or pull requests that need further clarification from the author GPG-Signing needed Pull requests that need GPG-Signing labels Dec 1, 2022
@datamweb
Copy link
Collaborator

datamweb commented Dec 1, 2022

Please pay attention to what Kenjis said.
#520 (comment)

Copy link
Member

@kenjis kenjis left a comment

Choose a reason for hiding this comment

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

Commit messages are important. They communicate the intent of a specific change, concisely. They make it easier to review code, and to find out why a change was made if the code history is examined later.

The audience for your commit messages will be the codebase maintainers, any code reviewers, and debuggers trying to figure out when a bug might have been introduced.

Make your commit messages meaningful.

Commit messages are expected to be descriptive of why and what you changed specifically. Commit messages like "Fixes #1234" would be asked by the reviewer to be revised.

@kpeu3u kpeu3u changed the title Fix query builder when updating last active date refactor: updateActiveDate query builder Dec 1, 2022
@MGatner MGatner requested a review from kenjis December 4, 2022 12:08
@MGatner
Copy link
Member

MGatner commented Dec 4, 2022

@kenjis Waiting on you

@kenjis kenjis merged commit cf21770 into codeigniter4:develop Dec 4, 2022
@kenjis
Copy link
Member

kenjis commented Dec 4, 2022

Thank you all.

@kpeu3u kpeu3u deleted the fix-set-last-active branch December 5, 2022 07:30
@kenjis kenjis added refactor Pull requests that refactor code and removed enhancement New feature or request labels Dec 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Pull requests that refactor code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants