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

[AL-587] Adding env:commit and env:code-log commands #1250

Merged
merged 2 commits into from
Oct 1, 2016

Conversation

marktheunissen
Copy link
Contributor

No description provided.

@pantheon-mentionbot
Copy link

@marktheunissen, thanks for your PR! By analyzing the blame information on this pull request, we identified @TeslaDethray and @ronan to be potential reviewers

@marktheunissen marktheunissen changed the title Adding env:commit and env:code-log commands [AL-587] Adding env:commit and env:code-log commands Sep 26, 2016
$env = $site->environments->get($env_name);

// TODO: The 0.x command would prompt if there were no changed files, asking
// if the user wanted to commit anyway.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ari-gold What is the desired behavior if there is nothing to commit?

Copy link
Contributor

Choose a reason for hiding this comment

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

I can't tell why the 0.x command prompts in this way. For now, let's just go with:

"[warning] There is no code to commit on <site.env>."
or
"[warning] There is no code to commit."

like
screenshot 2016-09-29 01 47 07
."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

screen shot 2016-09-30 at 11 57 43

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ari-gold done

str_replace(
"\n",
'',
str_replace("\t", '', substr($log->get('message'), 0, 50))

Choose a reason for hiding this comment

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

str_replace(array("\n", "\t"), '' ...) could also work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah maybe I just copied it from the old code... :) We're porting the 0.x commands to 1.0

@marktheunissen marktheunissen self-assigned this Sep 26, 2016
@marktheunissen marktheunissen force-pushed the env_commit_and_log branch 2 times, most recently from 514ae24 to e2579e3 Compare September 26, 2016 17:53
@coveralls
Copy link

coveralls commented Sep 26, 2016

Coverage Status

Coverage remained the same at 22.972% when pulling e2579e3 on env_commit_and_log into cf0b87d on master.

@coveralls
Copy link

coveralls commented Sep 27, 2016

Coverage Status

Coverage remained the same at 22.972% when pulling 73869cb on env_commit_and_log into 30715ec on master.

@@ -31,7 +32,6 @@ public function deploy($site_env, $options = [
'cc' => false,
'updatedb' => false,])
{
// @TODO: Switch this to the standard site.env input format?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ronan I don't follow this comment - are you saying this is this not the standard site.env format?

@coveralls
Copy link

coveralls commented Sep 27, 2016

Coverage Status

Coverage remained the same at 22.972% when pulling 44de42b on env_commit_and_log into 30715ec on master.

Copy link
Contributor

@TeslaDethray TeslaDethray left a comment

Choose a reason for hiding this comment

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

The messy str_replace that @aleszoulek would be worth tidying up, but this looks good!

*
* @command env:code-log
*
* @param string $site_env Site & environment to show log for.
Copy link
Contributor

@TeslaDethray TeslaDethray Sep 27, 2016

Choose a reason for hiding this comment

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

I've just noticed that there aren't any labels on the fields. Please add some. Here's the syntax for quick reference:
https://github.com/pantheon-systems/terminus/blob/master/src/Commands/Upstream/UpdatesListCommand.php#L20-L26

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 do, thanks!

@coveralls
Copy link

coveralls commented Sep 30, 2016

Coverage Status

Coverage remained the same at 23.539% when pulling 2f4c31c on env_commit_and_log into 80620b8 on master.

@coveralls
Copy link

coveralls commented Sep 30, 2016

Coverage Status

Coverage remained the same at 23.539% when pulling 9ecf529 on env_commit_and_log into 80620b8 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 23.557% when pulling bbb31e1 on env_commit_and_log into 07937db on master.

1 similar comment
@coveralls
Copy link

coveralls commented Oct 1, 2016

Coverage Status

Coverage remained the same at 23.557% when pulling bbb31e1 on env_commit_and_log into 07937db on master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants