-
-
Notifications
You must be signed in to change notification settings - Fork 28
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
feat: support release commits (and improve tests) #62
Conversation
|
ae9419b
to
6f81fc5
Compare
include_native_sources: true | ||
upload_source_maps: true | ||
auth_token: t | ||
auth_token: t # TODO: support not specifying this, let sentry-cli use the value it can find in its configs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem was that this plugin could not log a proper error, sentry CLI outputs too much stuff when something goes wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That can be solved too. I just didn't want to do this unrelated thing in the same PR.
Also relates to #23
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but I didn't want to during the hack week, we can leave this open and fix it later.
My message was more about giving context on why it's like this.
test/plugin_test.dart
Outdated
}); | ||
|
||
test('works with pubspec', () async { | ||
// TODO: because `url` param affects all commands, make it a test-group argument and run all test cases with/without it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something that we should address in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
didn't want to blow this up too much but since you've already seen tests without that change, I've done it now.
@vaind Amend the README, https://github.com/getsentry/sentry-dart-plugin/blob/main/README.md#configuration-optional |
📜 Description
💡 Motivation and Context
💚 How did you test it?
added unit tests and manually on a personal project
📝 Checklist
🔮 Next steps