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

Dart fixes #466

Merged
merged 4 commits into from
Jul 23, 2020
Merged

Dart fixes #466

merged 4 commits into from
Jul 23, 2020

Conversation

cvega
Copy link
Contributor

@cvega cvega commented Jul 23, 2020

Fixes issues with dart pointed out in #120

This change backs out the pub get || true in linter.sh for the following reasons:

  • pub get should only run once
    • pub-get should run before super-linter in it's own stage
  • I'm not seeing this approach with other linters in super-linter and prefer a standardized approach
  • Tests were not using pub get so they're were not testing the functionality/use case.

Proposed Changes

  1. Calling pub get on each lint even with || true is undesirable coupled with the fact it does not retain +x on copy when building Dockerfile (some how everything else in dart-sdk does?, not a super-linter issue).
  2. I left the glibc apk package laying around, cleaning up
  3. Update wiki to reflect the analysis_options.yaml file
  4. Doh, left the template out some how

Readiness Checklist

  • Label as breaking if this is a large fundamental change
  • Label as either automation, bug, documentation, enhancement, infrastructure, or performance

On second thought, I do not like this approach because it's asking `pub` questions it has already answered for each file, coupled with the fact that `|| true` means pub can fail for whatever reason. Looking at the other linters, and looking at the recent bug report in #120 I think it's safe to assume that pub, or any package manager can be called as a first step and then proceed to super lint.
- modified invalid file names (changed to anaylsis_options.yaml)
Noticed `glibc-${GLIBC_VERSION}.apk` was getting purged after and laying around in `/`.
@cvega cvega added the bug Something isn't working label Jul 23, 2020
@cvega cvega self-assigned this Jul 23, 2020
Copy link
Collaborator

@admiralAwkbar admiralAwkbar left a comment

Choose a reason for hiding this comment

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

Looks dope

@admiralAwkbar admiralAwkbar merged commit a5977bc into master Jul 23, 2020
@admiralAwkbar admiralAwkbar deleted the dart-fixes branch July 23, 2020 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants