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

add double to internal isValueModified check. bug #88 #94

Merged
merged 1 commit into from
Jan 10, 2023

Conversation

connorhu
Copy link
Collaborator

When you call a setter on a field with type double it mark the field modified even if nothing is changed.

bug #88

tests/models/Location2.php Outdated Show resolved Hide resolved
@connorhu
Copy link
Collaborator Author

connorhu commented Jan 10, 2023

@thePanz Do we want phpcs and php-cs-fixer github action and/or a project based config?
Something like that:

- name: Setup PHP
  uses: shivammathur/setup-php@v2
  with:
    php-version: '8.1'
    tools: cs2pr, phpcs

- name: Run phpcs
  run: phpcs -q --report=checkstyle lib | cs2pr

and https://github.com/OskarStark/php-cs-fixer-ga

@thePanz
Copy link
Member

thePanz commented Jan 10, 2023

@thePanz Do we want phpcs and php-cs-fixer github action and/or a project based config? Something like that:

Would be cool, to have php-cs-fixer for both symfony1 and doctrine1, but that's a topic for another PR ;-)

@thePanz thePanz merged commit b4749b5 into FriendsOfSymfony1:master Jan 10, 2023
@connorhu
Copy link
Collaborator Author

connorhu commented Jan 10, 2023

@thePanz Thank you for the merge! I will make a new PR about the php-cs-fixer when I solve one issue (we must check the changed files only)

@thePanz
Copy link
Member

thePanz commented Jan 10, 2023

@thePanz Thank you for the merge! I will make a new PR about the php-cs-fixer when I solve one issue (we must check the changed files only)

Would be cool to have both: the PR check, and a PR with the cs-fixes applied to all codebase. WDYT?
Would you please create an issue for that? thanks :)

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