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

Show diff for the dirty build. #3885

Merged
merged 2 commits into from
Oct 24, 2019
Merged

Show diff for the dirty build. #3885

merged 2 commits into from
Oct 24, 2019

Conversation

andreas-hs
Copy link
Contributor

@andreas-hs andreas-hs commented Oct 11, 2019

Fixes #3564

Changes proposed

Print diff information for the dirty build.

Steps to replicate the issue

  1. Run dirty build

Previous (bad) behavior, before applying PR

BLT showed only file was changed and not what actually changed in those files.
It can be difficult to debug what gone un.
Like this https://prnt.sc/phxpjb

Expected behavior, after applying PR and re-running test steps

BLT will show changes, like this https://prnt.sc/phxmyz

Additional details

Those changes can save a lot of time, which needed to resolved unclear bugs.

@@ -162,6 +162,13 @@ public function checkDirty(array $options = ['ignore-dirty' => FALSE]) {
$this->logger->warning("There are uncommitted changes on the source repository.");
}
else {
if ($options['verbose']) {
$this->taskExec('git diff --exit-code')
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd say git status is less verbose. In this case you probably care most about what files are changed, not the diff.

Copy link
Contributor Author

@andreas-hs andreas-hs Oct 14, 2019

Choose a reason for hiding this comment

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

Hi @alexxed !
Thank you for your review.
Yes git status is less verbose.
Therefore, it is not always clear what exactly has changed. I gave an example with settings.php
There are several behaviors that can lead to changes in the build.

  • If a Drupal installer is running, a line with the type of the installed profile may be added.
  • File permissions may change.
  • A line with blt is added to the end of the file

Not all such changes are understandable when viewing git status.
In addition, when an error is found, more information gives more power to solve it.

Is it make sense?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at the screenshot I see it's a CI, so it makes sense to get as much info as possible automatically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alexxed great, thanks!
What is the next step to processed it?

@danepowell danepowell merged commit 2cd4a5c into acquia:10.x Oct 24, 2019
@danepowell danepowell added 10.x Backport needed PRs needing to be backported Enhancement A feature or feature request labels Oct 24, 2019
danepowell pushed a commit that referenced this pull request Oct 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backport needed PRs needing to be backported Enhancement A feature or feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants