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

Enhancement: Add PHP-CS-Fixer #31

Merged
merged 3 commits into from
Aug 20, 2024
Merged

Conversation

OskarStark
Copy link
Contributor

@OskarStark OskarStark commented Aug 12, 2024

With this PR, everyone is able to execute make cs

@slunak best reviews commit by commit, as the first one introduces the config and the second one executes the PHP-CS-Fixer

@@ -1,28 +1,35 @@
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It also executes "composer normalize"

Comment on lines +88 to +96
'declare_strict_types' => false,
'void_return' => false,
'ordered_imports' => false,
'php_unit_test_case_static_method_calls' => false,
'strict_comparison' => false,
'yoda_style' => false,
'phpdoc_to_property_type' => false,
'phpdoc_summary' => false,
'nullable_type_declaration_for_default_null_value' => false,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will activate and execute them, once this PR is merged.

@@ -32,9 +32,17 @@ jobs:
extensions: "${{ env.REQUIRED_PHP_EXTENSIONS }}"
tools: "composer:v2"

- name: "Composer install"
uses: "ramsey/composer-install@v1"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

can be upgraded to v3 across the whole file afterwards

@slunak
Copy link
Owner

slunak commented Aug 14, 2024

Thanks @OskarStark , I will review later.

@OskarStark
Copy link
Contributor Author

Any news here?

*/
private $recipient;

/**
* @var GlanceDataFields Glance Data Fields.
* @var GlanceDataFields glance Data Fields
*/
Copy link
Owner

Choose a reason for hiding this comment

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

Is it possible to go through such changes, review them and move description to the top please? E.g.:

   /**
   * Glance Data Fields
   * 
   * @var GlanceDataFields
   */

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will check in the next days

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will be moved anyway when we add native types. From this point on it is superfluous for Cs-Fixer and it will be removed, resulting in the plain description as comment.

src/Api/Message/CustomSound.php Show resolved Hide resolved
@slunak
Copy link
Owner

slunak commented Aug 19, 2024

Any news here?

Sorry, busy couple of days, completely forgot about it. Overall I am ok with all the changes. Do we need to update CONTRIBUTING.md to reflect changes to cs-fixer or add more instructions in this regard?

@OskarStark
Copy link
Contributor Author

So ready to merge from my side

@slunak slunak merged commit b5037ce into slunak:master Aug 20, 2024
6 checks passed
@OskarStark
Copy link
Contributor Author

Will create follow up PRs soon 🤞🏻✌🏻

@slunak
Copy link
Owner

slunak commented Aug 20, 2024

Thank you @OskarStark :)

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.

3 participants