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

Cannot use auth token in .properties file and rest of config in pubspec.yaml #294

Open
buenaflor opened this issue Jan 22, 2025 · 5 comments · May be fixed by #295
Open

Cannot use auth token in .properties file and rest of config in pubspec.yaml #294

buenaflor opened this issue Jan 22, 2025 · 5 comments · May be fixed by #295
Assignees

Comments

@buenaflor
Copy link
Contributor

buenaflor commented Jan 22, 2025

User reported that they have an auth token in their properties file and the rest of the config in the pubspec but this didn't seem to work.

We do merge the config of the files so we should investigate if the merging is correct

@denrase
Copy link
Collaborator

denrase commented Jan 22, 2025

We do have test checking the merging AFAIK. Could they provide us with the config(s) that they have?

@buenaflor
Copy link
Contributor Author

@denrase https://github.com/getsentry/sentry-dart-plugin/blob/main/lib/src/utils/config-reader/config_reader.dart#L16C1-L42C1 I see, the issue is that while we do merge configs, we don't merge pubspec and properties, we only read either one of them.

I don't know if it makes sense or not to merge them as well, maybe? I'm not sure tbh

@buenaflor
Copy link
Contributor Author

Tested this locally so I can confirm this doesnt work

@buenaflor buenaflor moved this from Needs Investigation to Backlog in Mobile & Cross Platform SDK Jan 22, 2025
@buenaflor
Copy link
Contributor Author

buenaflor commented Jan 22, 2025

Since Flutter Sentry Wizard relies on this feature, we will implement it

but we have to consider the order when merging things. if pubspec and properties both have the same config, which one takes precedence?

@denrase denrase linked a pull request Jan 22, 2025 that will close this issue
5 tasks
@buenaflor buenaflor moved this from Backlog to In Progress in Mobile & Cross Platform SDK Jan 22, 2025
@buenaflor buenaflor moved this from In Progress to Needs Review in Mobile & Cross Platform SDK Jan 22, 2025
@denrase
Copy link
Collaborator

denrase commented Jan 23, 2025

@buenaflor Pubspec values takes precedence over properties, as pubspec was also chosen before properties before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Needs Review
Development

Successfully merging a pull request may close this issue.

2 participants