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

asim: remove extra new line in PrintSpanConfigConformanceList #111204

Merged
merged 1 commit into from
Sep 25, 2023

Conversation

wenyihu6
Copy link
Contributor

@wenyihu6 wenyihu6 commented Sep 25, 2023

Previously, PrintSpanConfigConformanceList included an extra line at the end.
Following the asim output convention, most output functions defer the decision
to include this extra line to caller (top level) rather than the callee
functions. This patch removes the extra line in PrintSpanConfigConformanceList.

Epic: None

Release Note: None

@blathers-crl
Copy link

blathers-crl bot commented Sep 25, 2023

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

Before a member of our team reviews your PR, I have some potential action items for you:

  • Please ensure your git commit message contains a release note.
  • When CI has completed, please ensure no errors have appeared.

I was unable to automatically find a reviewer. You can try CCing one of the following members:

  • A person you worked with closely on this PR.
  • The person who created the ticket, or a CRDB organization member involved with the ticket (author, commenter, etc.).
  • Join our community slack channel and ask on #contributors.
  • Try find someone else from here.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@blathers-crl blathers-crl bot added O-community Originated from the community X-blathers-untriaged blathers was unable to find an owner labels Sep 25, 2023
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@blathers-crl
Copy link

blathers-crl bot commented Sep 25, 2023

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

  • Please ensure your git commit message contains a release note.
  • When CI has completed, please ensure no errors have appeared.

I was unable to automatically find a reviewer. You can try CCing one of the following members:

  • A person you worked with closely on this PR.
  • The person who created the ticket, or a CRDB organization member involved with the ticket (author, commenter, etc.).
  • Join our community slack channel and ask on #contributors.
  • Try find someone else from here.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@wenyihu6 wenyihu6 marked this pull request as ready for review September 25, 2023 14:53
@wenyihu6 wenyihu6 requested a review from a team as a code owner September 25, 2023 14:53
@wenyihu6 wenyihu6 changed the title asim: remove empty line in PrintSpanConfigConformanceList asim: remove new line in PrintSpanConfigConformanceList Sep 25, 2023
@wenyihu6 wenyihu6 changed the title asim: remove new line in PrintSpanConfigConformanceList asim: remove extra new line in PrintSpanConfigConformanceList Sep 25, 2023
@blathers-crl
Copy link

blathers-crl bot commented Sep 25, 2023

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

  • Please ensure your git commit message contains a release note.
  • When CI has completed, please ensure no errors have appeared.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

Copy link
Collaborator

@kvoli kvoli left a comment

Choose a reason for hiding this comment

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

Do the existing randomized datadriven files need to be regenerated?

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @wenyihu6)


-- commits line 2 at r1:
There's an extra e on the end of PrintSpanConfigConformanceList

@blathers-crl
Copy link

blathers-crl bot commented Sep 25, 2023

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

  • Please ensure your git commit message contains a release note.
  • When CI has completed, please ensure no errors have appeared.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@wenyihu6
Copy link
Contributor Author

-- commits line 2 at r1:

Previously, kvoli (Austen) wrote…

There's an extra e on the end of PrintSpanConfigConformanceList

oops

@wenyihu6
Copy link
Contributor Author

The current randomized datadriven files don't have any failed assertions yet. The new PR has a few.

@wenyihu6 wenyihu6 requested a review from kvoli September 25, 2023 15:50
Previously, PrintSpanConfigConformanceList included an extra line at the end.
Following the asim output convention, most output functions defer the decision
to include this extra line to caller  (top level) rather than the callee
functions. This patch removes the extra line in PrintSpanConfigConformanceList.

Epic: None

Release Note: None
@blathers-crl
Copy link

blathers-crl bot commented Sep 25, 2023

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

  • Please ensure your git commit message contains a release note.
  • When CI has completed, please ensure no errors have appeared.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

Copy link
Collaborator

@kvoli kvoli left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @wenyihu6)

@kvoli
Copy link
Collaborator

kvoli commented Sep 25, 2023

bors r=kvoli

@craig
Copy link
Contributor

craig bot commented Sep 25, 2023

Build failed (retrying...):

@yuzefovich yuzefovich removed the X-blathers-untriaged blathers was unable to find an owner label Sep 25, 2023
@craig
Copy link
Contributor

craig bot commented Sep 25, 2023

Build succeeded:

@craig craig bot merged commit 9f1ee63 into cockroachdb:master Sep 25, 2023
8 checks passed
@wenyihu6 wenyihu6 deleted the main branch September 26, 2023 00:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-community Originated from the community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants