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.

Sign up for GitHub

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

Jump to bottom

Automation #635

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

Automation #635

wants to merge 5 commits into from

Conversation

ramiy
Copy link
Contributor

@ramiy ramiy commented Oct 11, 2016

Resolves #634

@jrfnl
Copy link
Contributor

jrfnl commented Oct 11, 2016

What are you actually trying to solve here ?
And who is the intended user of your script ?

@ramiy
Copy link
Contributor Author

ramiy commented Oct 11, 2016

The gulp script is trying to:

  1. Checks gettext function calls for missing or incorrect text domain (more info).
  2. Generate POT files for WordPress localization (more info).

But this is only the beginning, we can add other scripts.

@jrfnl
Copy link
Contributor

jrfnl commented Oct 11, 2016

That doesn't actually answer my questions.

GaryJones
GaryJones requested changes Oct 11, 2016
View reviewed changes
gulpfile.js
@@ -0,0 +1,61 @@
/**
* Give Gulp File
Copy link
Member

Choose a reason for hiding this comment

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

Incorrect comment.

class-tgm-plugin-activation.php
@@ -338,38 +338,38 @@ public function init() {
/* translators: %s: plugin name. */
Copy link
Member

Choose a reason for hiding this comment

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

These should be a separate PR please.

gulpfile.js
return gulp.src('**/*.php')
.pipe(sort())
.pipe(wpPot({
package: 'TGM Plugin Activation',
Copy link
Member

Choose a reason for hiding this comment

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

Please put these TGMPA-specific values into package.json, and have this file read from there.

gulpfile.js
------------------------------------- */
gulp.task('textdomain', function () {
var options = {
text_domain: 'tgmpa',
Copy link
Member

Choose a reason for hiding this comment

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

Again, read from package.json.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GaryJones GaryJones GaryJones requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
Status: Maybe Later Status: Reporter Feedback Needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ramiy @jrfnl @GaryJones