Skip to content

Conversation

@kelvinou01
Copy link
Contributor

Before this PR

When git diff fails, we don't tell the user the exit code and stderr/stdout, meaning the user has to dive in and debug.

After this PR

==COMMIT_MSG==
Improve logging when git diff fails
==COMMIT_MSG==

Possible downsides?

@changelog-app
Copy link

changelog-app bot commented Dec 1, 2025

Generate changelog in changelog/@unreleased

Type (Select exactly one)

  • Feature (Adding new functionality)
  • Improvement (Improving existing functionality)
  • Fix (Fixing an issue with existing functionality)
  • Break (Creating a new major version by breaking public APIs)
  • Deprecation (Removing functionality in a non-breaking way)
  • Migration (Automatically moving data/functionality to a new system)

Description

Improve logging when git diff fails

Check the box to generate changelog(s)

  • Generate changelog entry

@changelog-app
Copy link

changelog-app bot commented Dec 1, 2025

Successfully generated changelog entry!

Need to regenerate?

Simply interact with the changelog bot comment again to regenerate these entries.


📋Changelog Preview

💡 Improvements

  • Improve logging when git diff fails (#1490)

gradle/.DS_Store Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

How did this get added?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yikes, no idea!

esword
esword previously approved these changes Dec 1, 2025
@policy-bot policy-bot bot dismissed esword’s stale review December 1, 2025 10:40

Comment was edited

Preconditions.checkState(process.exitValue() == 0, "Expected return code of 0");

if (process.exitValue() != 0) {
throw new IllegalStateException("Expected return code of 0 but got " + process.exitValue()
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 use a RuntimeException here instead, IllegalStateException is more to signal an illegal state of the application

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotcha, i used illegalstate to maintain parity with the previous exception checkState throws, but a runtime exception does make more sense here

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.

4 participants