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

fix: deprecate set_output from GitHub workflow #2068

Merged
merged 6 commits into from
Dec 24, 2023

Conversation

EshaanAgg
Copy link
Contributor

Which problem is this PR solving?

Fixes #2067

Description of the changes

Removes the set-output command from the workflow

How was this change tested?

NA

Checklist

@EshaanAgg EshaanAgg requested a review from a team as a code owner December 24, 2023 17:09
@EshaanAgg EshaanAgg requested review from yurishkuro and removed request for a team December 24, 2023 17:09
Copy link

codecov bot commented Dec 24, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (33b22a4) 96.56% compared to head (c36481c) 96.56%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2068   +/-   ##
=======================================
  Coverage   96.56%   96.56%           
=======================================
  Files         255      255           
  Lines        7622     7622           
  Branches     1983     1983           
=======================================
  Hits         7360     7360           
  Misses        262      262           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yurishkuro yurishkuro added the changelog:ci Change related to continuous integration / testing label Dec 24, 2023
@yurishkuro
Copy link
Member

it's failing

Run is_dependabot=$(echo EshaanAgg | grep -o 'dependabot')
  is_dependabot=$(echo EshaanAgg | grep -o 'dependabot')
  echo "is_dependabot=$is_dependabot" >> $GITHUB_OUTPUT
  shell: /usr/bin/bash -e {0}

could it be that grep exits with error code?

@yurishkuro
Copy link
Member

yep

$ echo foo | grep bar

 12:14:55 🔴 

EshaanAgg and others added 4 commits December 24, 2023 22:55
## Which problem is this PR solving?
Fixes part of jaegertracing#1608

## Description of the changes
- Added the minimal types needed to fix the linting warnings
- Replaced `{}` with the `object` type

## How was this change tested?
- Ran `yarn lint` locally

## Checklist
- [X] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [X] I have signed all commits
- [X] I have added unit tests for the new functionality
- [X] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: Eshaan Aggarwal <96648934+EshaanAgg@users.noreply.github.com>
Signed-off-by: Eshaan Aggarwal <96648934+EshaanAgg@users.noreply.github.com>
Signed-off-by: Eshaan Aggarwal <96648934+EshaanAgg@users.noreply.github.com>
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Signed-off-by: Eshaan Aggarwal <96648934+EshaanAgg@users.noreply.github.com>
Signed-off-by: Eshaan Aggarwal <96648934+EshaanAgg@users.noreply.github.com>
@EshaanAgg
Copy link
Contributor Author

@yurishkuro Sorry for so many extraneous commits! This is the indented functionality right?

Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

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

This is much cleaner! Thanks.

@yurishkuro yurishkuro merged commit 4fefcac into jaegertracing:main Dec 24, 2023
10 checks passed
@EshaanAgg EshaanAgg deleted the work branch December 24, 2023 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:ci Change related to continuous integration / testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

check-label workflow: The set-output command is deprecated
2 participants