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

docs: Reduce excessive comments in code examples #984

Merged
merged 12 commits into from
Aug 6, 2020
Merged

docs: Reduce excessive comments in code examples #984

merged 12 commits into from
Aug 6, 2020

Conversation

kampalex
Copy link
Contributor

Fixes

Fixes #981

As mentioned, a lot of excessive comments are in example files and docs. I've reduced comments as much as possible.
Also:

  • Examples are made executable by loading existing file sendgrid-php.php in project root, instead of requiring an adjustment in the file to adjust path to vendor/autoload.php - which may not exists if composer install isn't executed after pulling library from Git repository.
  • Link classes (please report if unwanted!)
  • Fixed typos
  • Removed invisible whitespace in example files (removed by PHP CS Fixer I think)
  • Removed PHP close tag in sendgrid-php.php

Checklist

  • I acknowledge that all my contributions will be made under the project's license
  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the Contribution Guidelines and my PR follows them
  • I have titled the PR appropriately
  • I have 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 inline documentation to the code I modified

kampalex added 10 commits May 19, 2020 15:38
Updated code base from upstream repository
Update repository using sendgrid/master
Sync local repository using original
Update repository using sendgrid/master
Sync local repository using original
Sync local repository using original
Sync forked repository using original
PHP CS Fixer fixes
Docs: Import classes instead of namespace\class
Line length reductions
Prece(n)dence typos
Remove PHP tailing '?>' tags
Examples pending for migration: senderauthentication, contactdb
@thinkingserious thinkingserious added the status: code review request requesting a community code review or review from Twilio label Jun 24, 2020
README.md Outdated
// replacing <PATH TO> with the path to the sendgrid-php.php file,
// which is included in the download:
// https://github.com/sendgrid/sendgrid-php/releases
// Uncomment next line if you're not using a dependency loader (such as Composer)
Copy link
Contributor

Choose a reason for hiding this comment

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

First, a bit of context. We had a period where were getting a steady stream of issues at our support channel with confusion on installation, which virtually went away with this updated content. So I'm nervous about removing it entirely, but I do agree it's still verbose and likely not necessary for the majority of use cases.

Above this line, I think we should still include:
require 'vendor/autoload.php'; // If you're using Composer (recommended)

Then, something like:
// Comment out the above line and uncomment the next line if you're not using a dependency loader (such as Composer)

And for this part:

// download sendgrid-php.zip from the latest release here,	
// replacing <PATH TO> with the path to the sendgrid-php.php file,	
// which is included in the download:	
// https://github.com/sendgrid/sendgrid-php/releases

we can move those instructions to https://github.com/sendgrid/sendgrid-php/blob/master/README.md#install-package where we can then link from here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I went ahead and made the changes.

// Uncomment next line if you're not using a dependency loader (such as Composer)
// require_once '<PATH TO>/sendgrid-php.php';

use SendGrid\Mail\Mail;
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@thinkingserious thinkingserious added status: waiting for feedback waiting for feedback from the submitter type: community enhancement feature request not on Twilio's roadmap and removed status: code review request requesting a community code review or review from Twilio labels Jun 25, 2020
@childish-sambino childish-sambino removed the type: community enhancement feature request not on Twilio's roadmap label Jul 20, 2020
@thinkingserious thinkingserious changed the base branch from master to main July 28, 2020 14:34
@childish-sambino childish-sambino merged commit 7299d3d into sendgrid:main Aug 6, 2020
@childish-sambino childish-sambino removed the status: waiting for feedback waiting for feedback from the submitter label Aug 6, 2020
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.

Docs/Examples: reduce excessive comment headings
3 participants