chore: add pub analysis to CI #1408

Jordan-Nelson · 2022-02-25T21:08:22Z

Issue #, if available:

Description of changes:

Add a new step to CI to run pub analysis. This uses the package pana, which is what calculates the score on pub. The step will fail if the package is more than N points below the max score, where N is a threshold that we set
Enable step for all non interface packages, and set appropriate threshold
Fixed an issue in amplify_authenticator to get it to the max score

We should aim to get to the max score for all packages if possible, but right now this PR sets the threshold at the lowest it can for each package without failing.

Here is a link to a previous CI run where the amplift_flutter package threshold was too low (set to 10, but 20 points were missing). This failed and the logs show the score for the package.

Note: 120 is considered a "max" score for packages that don't support web, even though adding web support would bump the score to 130. It is still considered a "max" by pana.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Jordan-Nelson · 2022-02-25T21:51:16Z

packages/amplify_authenticator/lib/src/services/amplify_auth_service.dart

@@ -18,7 +18,6 @@ import 'dart:async';
 import 'package:amplify_auth_cognito/amplify_auth_cognito.dart';
 import 'package:amplify_flutter/amplify_flutter.dart';
 import 'package:collection/collection.dart';
-import 'package:collection/src/iterable_extensions.dart';


Removing this was needed to get the authenticator to a max pub score

I'm assuming it was superfluous and we had access to the extensions via collection.dart?

Jordan-Nelson · 2022-02-25T22:03:22Z

.circleci/config.yml

+              plugin:
+                [
+                  "amplify_authenticator",
+                  # TODO: enable other packages once pub score is addressed


amplify_authenticator is the only package with a max score right now.

Note: 120 is considered a max score for packages that don't support web, even though adding web support would bump the score to 130. It is still considered a "max" by pana,

It seems like the other packages lose points for, among other things, a dev dependency on unpublished package amplify_test. Any ideas for how to resolve that? Without fixing that, it seems we will either a) have to use only in authenticator b) change the exit-code-threshold to 10 (I think?) to basically ignore that loss of points.

I don't know off the top of my head. If we cannot figure out how to address that, we could set the threshold to 10, or set different threshold per package.

ok, makes sense. For this PR, I would either

a) merge it like this and figure it out when we add other plugins
b) do this now for any packages that can be published with threshold of 10 or 0.

I updated the PR to use a different threshold for each plugin. The one quirk with this is that the threshold is now in the CI job name. That means we will have to change the list of required jobs each time we adjust the threshold. I think thats fine though as it shouldn't be changed frequently.

codecov-commenter · 2022-02-25T22:33:51Z

Codecov Report

Merging #1408 (89f04da) into main (36fc97a) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1408   +/-   ##
=======================================
  Coverage   54.21%   54.21%           
=======================================
  Files         264      264           
  Lines       12010    12010           
=======================================
  Hits         6511     6511           
  Misses       5499     5499

Flag	Coverage Δ
android-unit-tests	`∅ <ø> (∅)`
flutter-unit-tests	`36.37% <ø> (ø)`
ios-unit-tests	`90.07% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ticator/lib/src/services/amplify_auth_service.dart	`0.00% <ø> (ø)`

ragingsquirrel3 · 2022-02-26T00:23:16Z

.circleci/config.yml

+              plugin:
+                [
+                  "amplify_authenticator",
+                  # TODO: enable other packages once pub score is addressed


ok, makes sense. For this PR, I would either

a) merge it like this and figure it out when we add other plugins
b) do this now for any packages that can be published with threshold of 10 or 0.

dnys1

I like this initiative! I think that striving for 120 is a bit arbitrary, but I think it will help us align with lint rules so I'm about it.

Thanks for adding this!

.circleci/config.yml

* chore: Update issue template to GH form (#1387) * chore: Update issue template to GH form * Add N/A for placeholders * Minor updates * Minor fixes * fix(amplify_authenticator): remove use of onInverseSurface to support older flutter versions (#1385) * remove use of onInverseSurface * set min flutter version to 2.0.0 * update fallback color * chore(amplify_authenticator): run unit tests for amplify_authenticator in CI (#1389) * fix(core): Implement Result in AtomicResult (#1393) * fix(core): Implement Result in AtomicResult * Fix AtomicResultTest * chore: Small QOS additions to bug report template (#1394) * fix(amplify_authenticator): allow multiple custom attributes in custom forms (#1377) * add initial category for ui customization * remove AuthState from the public API * remove auth bloc from public api * clean up public api, add docs * update for consistency, fix typos * change category name, fix typo * update install docs * update package description * update example project dir * uncomment example app imports * update template names * empty commit to trigger CI * update how runtime and custom fields are handled * move runtime fields to SignUpForm * fix signout button * Apply suggestions from code review Co-authored-by: Dillon Nys <24740863+dnys1@users.noreply.github.com> * rename includeDefaultFields, update sort * Update packages/amplify_authenticator/lib/src/widgets/form.dart Co-authored-by: Dillon Nys <24740863+dnys1@users.noreply.github.com> * make _includeDefaultFields private Co-authored-by: Dillon Nys <24740863+dnys1@users.noreply.github.com> * chore: authenticator v0.1.0 (#1402) * chore: authenticator v0.1.0 * Update packages/amplify_authenticator/pubspec.yaml Co-authored-by: Dillon Nys <24740863+dnys1@users.noreply.github.com> Co-authored-by: Dillon Nys <24740863+dnys1@users.noreply.github.com> * chore: adds API for auth integ tests + minor fixes (#1302) * chore(amplify_authenticator): add more detail to api docs for username (#1403) * add more detail to api docs for username * move docs to SignUpFormField * fix overflow in custom loyout example (#1401) * chore: rename docs dir to doc (#1406) * chore(amplify_api): clean up some code to limit analysis infos (#1404) * fix(datastore): delete default predicate causes deletion failure (#1409) * fix(datastore): delete default predicate causes deletion failure * fix save integration test * Release 4.0.1 version bump (#1418) * Version bump * fix changelog 0.4.0 entry * Add PR #s * chore: add pub analysis to CI (#1408) * add pana to ci * add missing colon * add install_melos step * move scope arg * move exec command to config * remove pana from melos * add remaining packages * address issue in authenticator package * disable failing packages * revert change to melos.yaml * add threshold * add quotes * escape quotes * escape << >> * escape <<< * update variable parsing * update parsing * remove trailing quote * add remaining packages * update variable name, add desc * adjust thresholds * decrease api threshold * chore: remove obsolete integration test driver files (#1395) * fix(amplify_api): model helpers query predicates correctly translates… (#1417) * chore(amplify_auth_cognito) federated plugin (#1349) * chore(auth): fix pubspec urls (#1424) Co-authored-by: Dillon Nys <24740863+dnys1@users.noreply.github.com> Co-authored-by: Jordan Nelson <nejrd@amazon.com> Co-authored-by: Travis Sheppard <tshepp@amazon.com> Co-authored-by: Jordan Nelson <Jordan.Ryan.Nelson@gmail.com> Co-authored-by: Dustin Noyes <dustin.noyes.dev@gmail.com> Co-authored-by: Kyle <kylcheng@gmail.com>

Jordan-Nelson added 8 commits February 25, 2022 16:07

add pana to ci

96bbd52

add missing colon

86fc629

add install_melos step

6563b70

move scope arg

d4c43a1

move exec command to config

a674a6e

remove pana from melos

a820264

add remaining packages

a7667b9

address issue in authenticator package

ca448c3

Jordan-Nelson commented Feb 25, 2022

View reviewed changes

disable failing packages

2dfa627

Jordan-Nelson commented Feb 25, 2022

View reviewed changes

revert change to melos.yaml

89f04da

Jordan-Nelson changed the title ~~[DRAFT] chore: add pub analysis to CI~~ chore: add pub analysis to CI Feb 25, 2022

Jordan-Nelson marked this pull request as ready for review February 25, 2022 22:09

Jordan-Nelson requested a review from a team as a code owner February 25, 2022 22:09

ragingsquirrel3 approved these changes Feb 26, 2022

View reviewed changes

dnys1 approved these changes Feb 26, 2022

View reviewed changes

Jordan-Nelson added 11 commits February 28, 2022 10:15

add threshold

aea3f8b

add quotes

e7db9e5

escape quotes

0c843b7

escape << >>

a384b7c

escape <<<

6c7c6bb

update variable parsing

789b932

update parsing

f9e701b

remove trailing quote

025289b

add remaining packages

966f1b8

update variable name, add desc

260bd02

adjust thresholds

fa7acfb

Jordan-Nelson commented Feb 28, 2022

View reviewed changes

.circleci/config.yml Outdated Show resolved Hide resolved

Jordan-Nelson added 2 commits February 28, 2022 11:37

Merge branch 'main' into ci/pana

6f88729

decrease api threshold

8d5731c

Jordan-Nelson merged commit ea88973 into aws-amplify:main Mar 1, 2022

Jordan-Nelson deleted the ci/pana branch March 1, 2022 17:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: add pub analysis to CI #1408

chore: add pub analysis to CI #1408

Jordan-Nelson commented Feb 25, 2022 •

edited

Loading

Jordan-Nelson Feb 25, 2022

haverchuck Feb 25, 2022

Jordan-Nelson Feb 25, 2022

Jordan-Nelson Feb 25, 2022

ragingsquirrel3 Feb 25, 2022

Jordan-Nelson Feb 25, 2022

ragingsquirrel3 Feb 26, 2022

Jordan-Nelson Feb 28, 2022

codecov-commenter commented Feb 25, 2022 •

edited

Loading

ragingsquirrel3 Feb 26, 2022

dnys1 left a comment

chore: add pub analysis to CI #1408

chore: add pub analysis to CI #1408

Conversation

Jordan-Nelson commented Feb 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Feb 25, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

dnys1 left a comment

Choose a reason for hiding this comment

Jordan-Nelson commented Feb 25, 2022 •

edited

Loading

codecov-commenter commented Feb 25, 2022 •

edited

Loading