Skip to content

chore: add style standards check#90

Merged
thinkingserious merged 16 commits intosendgrid:mainfrom
pangaunn:add-style-check
Oct 2, 2020
Merged

chore: add style standards check#90
thinkingserious merged 16 commits intosendgrid:mainfrom
pangaunn:add-style-check

Conversation

@pangaunn
Copy link
Contributor

@pangaunn pangaunn commented Oct 6, 2018 •
edited
Loading

Fixes

Fixes #87

Checklist

  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the [Contribution Guide] and my PR follows them.
  • I updated my branch with the master branch.
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation about the functionality in the appropriate .md file
  • I have added in line documentation to the code I modified

Short description of what this PR does:

  • Add short description in each function in header and add blank line between @param and @return.
  • Add travis to run phpcs.
  • Change toArray to public. This function is in the USAGE.md file I think it should be public.

If you have questions, please send an email to Sendgrid, or file a Github Issue in this repository.

@thinkingserious thinkingserious added the status: code review request requesting a community code review or review from Twilio label Oct 6, 2018
@SendGridDX
Copy link

SendGridDX commented Oct 6, 2018 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

peter279k
peter279k reviewed Oct 7, 2018
View reviewed changes
.travis.yml Outdated
- composer install --dev --no-interaction
- cd test
script:
- "php ../vendor/bin/phpcs -n --exclude=PEAR.Commenting.FileComment,PEAR.Commenting.ClassComment,PEAR.Commenting.FunctionComment ../lib/"
Copy link

@peter279k peter279k Oct 7, 2018

Choose a reason for hiding this comment

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

How about using the following phpcs command to check the PSR-2 coding style?

./vendor/bin/phpcs -n --standard=PSR2 lib/ test/

I think this command I suggest is good enough to use :).

Copy link
Contributor Author

@pangaunn pangaunn Oct 8, 2018

Choose a reason for hiding this comment

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

Thank you for review but In CONTRIBUTING.md it's using PEAR standard. I will add /test in script.

Copy link

@peter279k peter279k Oct 8, 2018

Choose a reason for hiding this comment

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

Mm....The coding standard is different from sendgrid-php. I think it also uses the PSR-2 coding style.

If this repo uses PEAR coding style, this comment I write will not be concerned :).

@devchas devchas added difficulty: medium fix is medium in difficulty hacktoberfest type: twilio enhancement feature request on Twilio's roadmap labels Oct 17, 2018
@pangaunn pangaunn mentioned this pull request Oct 26, 2018
Merged
6 tasks
@childish-sambino childish-sambino removed the type: twilio enhancement feature request on Twilio's roadmap label Jul 20, 2020
@thinkingserious thinkingserious changed the base branch from master to main July 28, 2020 14:37
@thinkingserious
Copy link
Contributor

thinkingserious commented Oct 1, 2020

Hello @pangaunn,

Apologies for the delayed code review.

I attempted to resolve the conflicts, but I do not have permission to push to your branch. Would you mind resolving the conflicts or granting me permission to push to your branch?

Thanks!

Elmer

@pangaunn
Copy link
Contributor Author

pangaunn commented Oct 1, 2020

Hello @thinkingserious I just send you an invite please check.

@thinkingserious thinkingserious changed the title Add style standards check chore: add style standards check Oct 2, 2020
@thinkingserious thinkingserious merged commit 02c9f04 into sendgrid:main Oct 2, 2020
@thinkingserious thinkingserious deleted the add-style-check branch October 2, 2020 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thinkingserious thinkingserious thinkingserious approved these changes

+1 more reviewer

@peter279k peter279k peter279k left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

difficulty: medium fix is medium in difficulty status: code review request requesting a community code review or review from Twilio

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Require conformance to style standards

6 participants

@pangaunn @SendGridDX @thinkingserious @peter279k @devchas @childish-sambino