Skip to content

Conversation

jiachengxu
Copy link
Contributor

@jiachengxu jiachengxu commented Dec 21, 2021

What this PR does:
This PR adds length and limit to labelNameTooLongError and labelValueTooLongError, so it will be easier to know what values should be set.

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Copy link
Contributor

@bboreham bboreham left a comment

Choose a reason for hiding this comment

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

Seems reasonable, thanks!

CHANGELOG.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: CHANGELOG is for admins, not for developers, so it is better to use English than type names.

Copy link
Contributor Author

@jiachengxu jiachengxu Dec 22, 2021

Choose a reason for hiding this comment

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

Oh, I see. How about:
Add length and limit to error messages when label names and values are too long

Copy link
Contributor

Choose a reason for hiding this comment

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

Great!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@jiachengxu jiachengxu changed the title Add length and limit to labelNameTooLongError and labelValueTooLongError Add length and limit to error messages when label names and values are too long Dec 23, 2021
@jiachengxu
Copy link
Contributor Author

Hi @bboreham, anything I need to do to get this PR merged? Also, the integration test failed but I guess this is not because of this PR?

@alvinlin123 alvinlin123 enabled auto-merge (squash) January 21, 2022 05:17
@alvinlin123 alvinlin123 merged commit 6b93903 into cortexproject:master Jan 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants