Skip to content

Conversation

denrase
Copy link
Collaborator

@denrase denrase commented Jul 15, 2021

Overview

  • Introduce sentry_upload_dif action.

Testing

  • Created rspec tests, testing the parameters and how they translate to sentry-cli parameters.
  • Tested with an iOS application that has sentry setup to upload dsym files using upload-dif action.

Relates to #86

@denrase denrase self-assigned this Jul 15, 2021
@denrase denrase changed the title feat: Action for sentry-cli upload-dif feat: Add sentry-cli upload-dif action Jul 15, 2021
@denrase denrase requested a review from philipphofmann July 15, 2021 09:09
@denrase denrase marked this pull request as ready for review July 15, 2021 09:09
@denrase denrase removed their assignment Jul 15, 2021
Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

RSLGTM

# Conflicts:
#	CHANGELOG.md
@github-actions
Copy link
Contributor

github-actions bot commented Jul 22, 2021

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 34a1d89

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

LGTM

@philipphofmann philipphofmann linked an issue Jul 27, 2021 that may be closed by this pull request
@denrase denrase merged commit 6203fc5 into master Jul 27, 2021
@denrase denrase deleted the feat/upload-dif branch July 27, 2021 09:59
Copy link

@flub flub 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 adding this? dumb question, but is this available to users yet or does it need to go through a release process first?

@flub
Copy link

flub commented Jul 29, 2021

Thanks for adding this?

I clearly know how to use a keyboard, that was meant to be !, not ?. 😬

@philipphofmann
Copy link
Member

Thanks for adding this? dumb question, but is this available to users yet or does it need to go through a release process first?

We need to do a release, but this is just a matter of minutes.

@flub
Copy link

flub commented Jul 29, 2021

Thanks for adding this? dumb question, but is this available to users yet or does it need to go through a release process first?

We need to do a release, but this is just a matter of minutes.

great, so we can include it in the docs already. but let's wait for the default of the path first i guess

@lhunath
Copy link

lhunath commented Aug 12, 2021

[01:09:28]: error: Found argument '--include_sources' which wasn't expected, or isn't valid in this context

As also mentioned here: 6203fc5#r54278929
This is incorrectly using underscores instead of dashes for sentry-cli arguments.

@lhunath
Copy link

lhunath commented Aug 12, 2021

Also (as mentioned here 6203fc5#commitcomment-54278944), sentry-cli -h doesn't even mention upload-dsym anymore; why is this PR adding an upload-dif action instead of renaming the old upload-dsym action? The result is now this plugin is overloaded and provides a duplicated outdated target and both need to be maintained.

@denrase
Copy link
Collaborator Author

denrase commented Aug 16, 2021

@lhunath Thanks for the feedback, we will look into this.

@lhunath
Copy link

lhunath commented Aug 17, 2021

@lhunath Thanks for the feedback, we will look into this.

You're welcome. I've made some proposals in #95 and PR #96 - hopefully it's useful.

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.

Add support for upload-dif
5 participants