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

Throw error if an expected release tag cannot be found, improve logs #25

Conversation

dgellow
Copy link
Member

@dgellow dgellow commented Sep 1, 2023

If a version is mentioned in the manifest, but the release tag has been deleted manually, the current implementation reports the issue with warning logs at the beginning of the process.

In the case of Stainless, a missing release tag is likely to be due to a problem in the release process we would want to fix. So instead of a warning log this pull request throws an error with a meaningful error message.

I also cleaned up a bunch of logs that I found confusing and/or too noisy when I was debugging timeouts with sink-java-public (caused by deleted releases and tags).

See full context here and here.

@dgellow dgellow changed the title Improve logging and throw error if no release/tag can be found for version in manifest Throw error if an expected release tag cannot be found, improve logs Sep 6, 2023
@dgellow dgellow marked this pull request as ready for review September 6, 2023 10:49
this.logger.warn(
`Expected ${expectedReleases} releases, only found ${releasesFound}`
);
const errMessage = `Expected to find ${expectedReleases} releases, but ${

Choose a reason for hiding this comment

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

I am still not sure about this. This will break customers and they most likely won't know what to do. Can we remedy it ourselves? If only one version is missing, we can maybe ignore it (and maybe generate a duplicate version)? Only when more than one is missing would we error out?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, let's keep this PR open for now, it's nothing urgent, we can change it later.

@dgellow dgellow closed this Oct 3, 2023
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.

2 participants