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

feat(ado/infra): remove unneeded encoding module from webpack #759

Merged
merged 3 commits into from
Aug 12, 2021

Conversation

karanbirsingh
Copy link
Contributor

@karanbirsingh karanbirsingh commented Aug 5, 2021

Details

This PR follows up from #752 (comment)

It tells webpack to stop including the encoding module. We don't use this code path & encoding is brought in from a devDependency of lerna.

We use the strategy in this comment: node-fetch/node-fetch#412 (comment)

Motivation

Stop including a package we don't use

Context

Pull request checklist

  • Addresses an existing issue: Fixes #0000
  • Added relevant unit test for your changes. (yarn test)
  • Verified code coverage for the changes made. Check coverage report at: <rootDir>/test-results/unit/coverage
  • Ran precheckin (yarn precheckin)
  • (after PR created) The Accessibility Checks (pull_request) check should fail. All other checks should pass.

@karanbirsingh karanbirsingh requested a review from a team as a code owner August 5, 2021 22:51
@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2021

Accessibility Insights Accessibility Insights Action

  • URLs: 1 URL(s) failed, 3 URL(s) passed, and 0 were not scannable
  • Rules: 6 check(s) failed, 11 check(s) passed, and 38 were not applicable
  • Download the Accessibility Insights artifact to view the detailed results of these checks

Failed instances

  • 3 × label: Ensures every form element has a label
  • 1 × html-has-lang: Ensures every HTML document has a lang attribute
  • 1 × image-alt: Ensures <img> elements have alternate text or a role of none or presentation
  • 1 × input-image-alt: Ensures <input type="image"> elements have alternate text

This scan used axe-core 4.2.1 with Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/86.0.4240.75 Safari/537.36.

@codecov-commenter
Copy link

codecov-commenter commented Aug 5, 2021

Codecov Report

Merging #759 (12fe2f9) into main (eae20c7) will decrease coverage by 0.66%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #759      +/-   ##
==========================================
- Coverage   94.34%   93.68%   -0.67%     
==========================================
  Files          26       27       +1     
  Lines         566      570       +4     
  Branches       54       54              
==========================================
  Hits          534      534              
- Misses         32       36       +4     
Impacted Files Coverage Δ
packages/ado-extension/src/index.ts 0.00% <0.00%> (ø)

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 0b9142b...12fe2f9. Read the comment docs.

@AhmedAbdoOrtiga
Copy link
Contributor

packages/gh-action/dist/index.js is still in the branch, please merge with main

@karanbirsingh karanbirsingh merged commit f083c7a into main Aug 12, 2021
@karanbirsingh karanbirsingh deleted the remove-unneeded-module branch August 12, 2021 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants