Fix: Read all environment variables in sentry #375

ueman · 2021-03-22T08:38:51Z

📜 Description

Environment variables are now read in sentry.
This also includes #376

💡 Motivation and Context

#306

💚 How did you test it?

I've written new tests.

📝 Checklist

🔮 Next steps

marandaneto · 2021-03-22T08:41:43Z

@ueman is PR title a typo? sentry_flutter, we should read from sentry package, so it'd work for non mobile apps

ueman · 2021-03-22T08:42:44Z

@ueman is PR title a typo? sentry_flutter, we should read from sentry package, so it'd work for non mobile apps

Yep, that was a typo

dart/lib/src/environment_variables.dart

dart/lib/src/sentry.dart

dart/lib/src/sentry_options.dart

dart/lib/src/platform_checker.dart

flutter/lib/src/default_integrations.dart

ueman · 2021-03-22T09:52:45Z

I've opened an issue for failing Flutter beta builds here: #377

CHANGELOG.md

Co-authored-by: Manoel Aranda Neto <[email protected]>

codecov-io · 2021-03-22T15:09:12Z

Codecov Report

Merging #375 (b9169f3) into main (0d775f1) will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #375      +/-   ##
==========================================
+ Coverage   90.19%   90.25%   +0.05%     
==========================================
  Files          51       52       +1     
  Lines        1642     1652      +10     
==========================================
+ Hits         1481     1491      +10     
  Misses        161      161

Impacted Files	Coverage Δ
dart/lib/src/sentry_options.dart	`81.25% <ø> (ø)`
dart/lib/src/environment_variables.dart	`100.00% <100.00%> (ø)`
dart/lib/src/platform_checker.dart	`75.00% <100.00%> (+15.00%)`	⬆️
dart/lib/src/sentry.dart	`85.71% <100.00%> (-0.96%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0d775f1...b9169f3. Read the comment docs.

flutter/lib/src/default_integrations.dart

dart/lib/src/environment_variables.dart

flutter/lib/src/default_integrations.dart

CHANGELOG.md

flutter/lib/src/sentry_flutter.dart

…bles

dart/test/environment_test.dart

dart/lib/src/environment_variables.dart

dart/lib/src/sentry.dart

bruno-garcia · 2021-03-25T02:54:07Z

Removed the #376 from the title

…bles

ueman · 2021-03-29T06:35:39Z

@marandaneto I fixed the last missing bits. Please let me know if it is now good to go.

marandaneto · 2021-03-29T07:32:16Z

dart/lib/src/platform_checker.dart

@@ -1,3 +1,5 @@
+import 'sentry_options.dart';


maybe we move defaultEnvironment to EnvironmentVariables instead

marandaneto · 2021-03-29T07:32:51Z

dart/lib/src/platform_checker.dart

@@ -17,4 +19,18 @@ class PlatformChecker {
  bool isProfileMode() {
    return const bool.fromEnvironment('dart.vm.profile', defaultValue: false);
  }
+
+  /// This can be set as [SentryOptions.environment]
+  String get environment {


should this be a method instead? quite a few conditions to be just a property, maybe we move hardcoded values to EnvironmentVariables instead

dart/lib/src/sentry.dart

marandaneto · 2021-03-29T07:37:12Z

dart/test/environment_test.dart

+      expect(options.dist, 'foo');
+    });
+
+    test('No options are set', () async {


maybe let's improve the test name, something like, SentryOptions are overridden by environment

marandaneto · 2021-03-29T07:39:51Z

flutter/test/default_integrations_test.dart

@@ -3,10 +3,12 @@ import 'package:flutter/services.dart';
 import 'package:flutter/widgets.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:mockito/mockito.dart';
+import 'package:package_info_plus/package_info_plus.dart';


merge main into this branch to not show the diff of non related stuff

marandaneto · 2021-03-29T07:40:48Z

@marandaneto I fixed the last missing bits. Please let me know if it is now good to go.

made a few small suggestions but other than that LGTM

marandaneto

LGTM

…bles

ueman added 4 commits March 22, 2021 09:33

Correctly read environment variables

6dd3619

Changelog

95a4d8a

Remove env vars from sentry_flutter

61fbabc

Docs & Changelog

3de7b95

ueman changed the title ~~Fix: Read all environment variables in sentry_flutter~~ Fix: Read all environment variables in sentry_dart Mar 22, 2021

Calm Linter

9ecebf3

ueman marked this pull request as ready for review March 22, 2021 08:45

ueman requested review from bruno-garcia and marandaneto as code owners March 22, 2021 08:45

marandaneto reviewed Mar 22, 2021

View reviewed changes

dart/lib/src/environment_variables.dart Outdated Show resolved Hide resolved

marandaneto reviewed Mar 22, 2021

View reviewed changes

dart/lib/src/sentry.dart Outdated Show resolved Hide resolved

marandaneto reviewed Mar 22, 2021

View reviewed changes

dart/lib/src/sentry_options.dart Show resolved Hide resolved

marandaneto reviewed Mar 22, 2021

View reviewed changes

dart/lib/src/platform_checker.dart Outdated Show resolved Hide resolved

marandaneto reviewed Mar 22, 2021

View reviewed changes

flutter/lib/src/default_integrations.dart Outdated Show resolved Hide resolved

ueman added 2 commits March 22, 2021 10:10

Hopefully fix failing tests

ccb6c74

Implement PR feedback

ecf2750

ueman force-pushed the fix/environment-variables branch from c1bfc93 to ecf2750 Compare March 22, 2021 09:33

Another try at fixing the failing tests

f37fc9c

ueman force-pushed the fix/environment-variables branch from ee82b3d to f37fc9c Compare March 22, 2021 09:39

This was referenced Mar 22, 2021

Fix: Set SentryOptions.debug in sentry #376

Merged

Flutter Beta builds are broken on CI #377

Closed

marandaneto reviewed Mar 22, 2021

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

ueman changed the title ~~Fix: Read all environment variables in sentry_dart~~ Fix: Read all environment variables in sentry Mar 22, 2021

Update CHANGELOG.md

b9169f3

Co-authored-by: Manoel Aranda Neto <[email protected]>

marandaneto reviewed Mar 22, 2021

View reviewed changes

flutter/lib/src/default_integrations.dart Outdated Show resolved Hide resolved

marandaneto reviewed Mar 22, 2021

View reviewed changes

flutter/lib/src/default_integrations.dart Outdated Show resolved Hide resolved

marandaneto reviewed Mar 23, 2021

View reviewed changes

dart/lib/src/environment_variables.dart Outdated Show resolved Hide resolved

marandaneto reviewed Mar 23, 2021

View reviewed changes

flutter/lib/src/default_integrations.dart Outdated Show resolved Hide resolved

marandaneto reviewed Mar 23, 2021

View reviewed changes

CHANGELOG.md Show resolved Hide resolved

marandaneto reviewed Mar 23, 2021

View reviewed changes

flutter/lib/src/sentry_flutter.dart Show resolved Hide resolved

ueman added 2 commits March 23, 2021 09:35

Merge remote-tracking branch 'origin/main' into fix/environment-varia…

adcb9ac

…bles

Only override release and dist if not existing

13d5e8f

ueman changed the title ~~Fix: Read all environment variables in sentry~~ Fix: Read all environment variables in sentry and #375 Mar 23, 2021

ueman changed the title ~~Fix: Read all environment variables in sentry and #375~~ Fix: Read all environment variables in sentry and #376 Mar 23, 2021

Apply PR feedback

ccb0a69

marandaneto reviewed Mar 23, 2021

View reviewed changes

dart/test/environment_test.dart Show resolved Hide resolved

Merge branch 'main' into fix/environment-variables

7fea45b

bruno-garcia reviewed Mar 24, 2021

View reviewed changes

dart/lib/src/environment_variables.dart Show resolved Hide resolved

dart/lib/src/sentry.dart Show resolved Hide resolved

bruno-garcia changed the title ~~Fix: Read all environment variables in sentry and #376~~ Fix: Read all environment variables in sentry Mar 25, 2021

ueman mentioned this pull request Mar 25, 2021

Set debug true as first thing in the SDKs #384

Closed

ueman added 3 commits March 25, 2021 15:24

add teardown

52af9c7

Merge remote-tracking branch 'origin/main' into fix/environment-varia…

9aa6633

…bles

Fix path

f2b320b

marandaneto reviewed Mar 29, 2021

View reviewed changes

dart/lib/src/sentry.dart Outdated Show resolved Hide resolved

marandaneto reviewed Mar 29, 2021

View reviewed changes

marandaneto approved these changes Mar 29, 2021

View reviewed changes

ueman added 2 commits March 29, 2021 10:17

Merge remote-tracking branch 'origin/main' into fix/environment-varia…

a3c31eb

…bles

PR feedback

3b83d90

ueman merged commit de0a2ce into main Mar 29, 2021

ueman deleted the fix/environment-variables branch March 29, 2021 09:01

Uh oh!

Fix: Read all environment variables in sentry #375

Fix: Read all environment variables in sentry #375

Uh oh!

Conversation

ueman commented Mar 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

🔮 Next steps

Uh oh!

marandaneto commented Mar 22, 2021

Uh oh!

ueman commented Mar 22, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ueman commented Mar 22, 2021

Uh oh!

Uh oh!

codecov-io commented Mar 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bruno-garcia commented Mar 25, 2021

Uh oh!

ueman commented Mar 29, 2021

Uh oh!

marandaneto Mar 29, 2021

Choose a reason for hiding this comment

Uh oh!

marandaneto Mar 29, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

marandaneto Mar 29, 2021

Choose a reason for hiding this comment

Uh oh!

marandaneto Mar 29, 2021

Choose a reason for hiding this comment

Uh oh!

marandaneto commented Mar 29, 2021

Uh oh!

marandaneto left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ueman commented Mar 22, 2021 •

edited

Loading

codecov-io commented Mar 22, 2021 •

edited

Loading