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 dashes (-) instead of 0 in the table #443

Merged
merged 8 commits into from Dec 12, 2023

Conversation

GordonBeeming
Copy link
Contributor

@GordonBeeming GordonBeeming commented Nov 7, 2023

Summary

Motivation

Show dashes (-) instead of 0 in the table
#442

Technical

Testing

Test Types

  • Unit tests
  • Manual tests

Unit Test Coverage

Other Test Details

Screenshots

@GordonBeeming GordonBeeming requested a review from a team as a code owner November 7, 2023 02:13
@GordonBeeming
Copy link
Contributor Author

@microsoft-github-policy-service agree

@GordonBeeming
Copy link
Contributor Author

This should be good to accept now 😅

@GordonBeeming
Copy link
Contributor Author

@muiriswoulfe do you mind taking a look at this PR, and see if it's something you'd want in the action 🙂

@muiriswoulfe
Copy link
Member

@muiriswoulfe do you mind taking a look at this PR, and see if it's something you'd want in the action 🙂

I was away, but I'll take a look this week.

Copy link
Member

@muiriswoulfe muiriswoulfe left a comment

Choose a reason for hiding this comment

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

Thanks for your changes. Overall, this looks good, but I've just left a few small comments.

src/task/src/pullRequests/pullRequestComments.ts Outdated Show resolved Hide resolved
src/task/task.json Outdated Show resolved Hide resolved
@muiriswoulfe muiriswoulfe added the enhancement New feature or request label Dec 11, 2023
@GordonBeeming
Copy link
Contributor Author

Thanks @muiriswoulfe, I've done the updates requested 😊

@GordonBeeming
Copy link
Contributor Author

It looks like there's token issues at the moment 😓

@muiriswoulfe
Copy link
Member

It looks like there's token issues at the moment 😓

The issue is actually this one – it's a known issue with GitHub: openmainframeproject/feilong#367

I'll need to create a local copy to validate this.

Copy link
Member

@muiriswoulfe muiriswoulfe left a comment

Choose a reason for hiding this comment

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

I've now run these changes locally via another PR, but I'd prefer if we merged your changes as you are the author not me and I don't intend to claim credit for this :)

One change you might have seen that the pipeline made is that it ran npm run build:package. Since the pipeline can't run here, can you run that manually, commit the changes, and then I'll override the build and merge to main. Thanks!

@GordonBeeming
Copy link
Contributor Author

GordonBeeming commented Dec 12, 2023

@muiriswoulfe Done - Thanks ☺️

@muiriswoulfe muiriswoulfe merged commit 672351a into microsoft:main Dec 12, 2023
2 checks passed
@GordonBeeming GordonBeeming deleted the issue-442 branch December 12, 2023 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants