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

ENH Add Nice to DBField #11184

Merged
merged 1 commit into from
Mar 27, 2024
Merged

ENH Add Nice to DBField #11184

merged 1 commit into from
Mar 27, 2024

Conversation

lekoala
Copy link
Contributor

@lekoala lekoala commented Mar 21, 2024

Description

Adding a Nice method on the base DBField class simplify code down the line and avoid extra method_exists checks and make static analysis tool happy

Manual testing steps

Classes types against DBField do not complain anymore about Nice not being present

Issues

Pull request checklist

  • The target branch is correct
  • All commits are relevant to the purpose of the PR (e.g. no debug statements, unrelated refactoring, or arbitrary linting)
    • Small amounts of additional linting are usually okay, but if it makes it hard to concentrate on the relevant changes, ask for the unrelated changes to be reverted, and submitted as a separate PR.
  • The commit messages follow our commit message guidelines
  • The PR follows our contribution guidelines
  • Code changes follow our coding conventions
  • This change is covered with tests (or tests aren't necessary for this change)
  • Any relevant User Help/Developer documentation is updated; for impactful changes, information is added to the changelog for the intended release
  • CI is green

@GuySartorelli
Copy link
Member

Please target the 5 branch (new methods should be included in minor releases, not patches) and tick all the boxes that apply in the PR description.

The actual code looks good to me. Nice and simple.

@lekoala lekoala changed the base branch from 5.1 to 5 March 22, 2024 08:20
@GuySartorelli
Copy link
Member

GuySartorelli commented Mar 24, 2024

It looks like you've inadvertently pulled in some commits that aren't related to this PR, and have additional commits to cancel them out - can you please instead make sure this PR only has the commits that are relevant to the change being made?
The high-level overview of how I recommend doing that is:

  1. Delete your local (not remote!) branch
  2. Switch to the 5 branch
  3. Create a new local branch with the same name as the original PR branch (i.e. patch-54 in this case)
  4. Cherry-pick the relevant commits
  5. Force-push the branch (it'll override the remote branch with the same name, thereby updating the PR)

More detailed instructions are in this gist

Please also tick the relevant boxes as per #11184 (comment)

Fixes silverstripe#11169

(cherry picked from commit 2c35b8d)
@lekoala
Copy link
Contributor Author

lekoala commented Mar 27, 2024

@GuySartorelli done, i'm getting better at this whole git branch management thing ;)

@GuySartorelli
Copy link
Member

That's perfect, thank you!

@GuySartorelli GuySartorelli merged commit 46c5f6d into silverstripe:5 Mar 27, 2024
15 checks passed
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.

2 participants