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

refactor: reduce imports for react-sytax-highlighter #665

Merged
merged 3 commits into from Feb 2, 2018

Conversation

efloden
Copy link
Member

@efloden efloden commented Jan 17, 2018

JIRA issue URL: N/A

Reduces the import footprint of the react-syntax-highlighter significantly by only importing the xml language. (which is all we are using at the moment)

Checklist

  • JIRA link
  • Check target branch
  • Make sure all commit statuses are green or otherwise documented reasons to ignore
  • QA needs to evaluate against the JIRA ticket
  • Changed files and commits make sense for this PR

See Zanata Development Guidelines more for information.


This template can be updated in .github/PULL_REQUEST_TEMPLATE.md


This change is Reviewable

@alex-sl-eng
Copy link
Member

:lgtm:


Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks broke.


Comments from Reviewable

@alex-sl-eng
Copy link
Member

@efloden please check the failed jenkins build.

@codecov
Copy link

codecov bot commented Jan 21, 2018

Codecov Report

Merging #665 into master will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #665      +/-   ##
============================================
+ Coverage     33.14%    33.2%   +0.06%     
- Complexity     5775    11568    +5793     
============================================
  Files          1575     3152    +1577     
  Lines         62267   124752   +62485     
  Branches       7351    14745    +7394     
============================================
+ Hits          20638    41425   +20787     
- Misses        39706    79454   +39748     
- Partials       1923     3873    +1950
Impacted Files Coverage Δ Complexity Δ
...app/editor/components/TransUnitTranslationPanel.js 13.88% <100%> (+13.88%) 0 <0> (ø) ⬇️
...ain/java/org/zanata/rest/review/ReviewService.java 72% <0%> (-12.22%) 7% <0%> (+1%)
...n-util/src/main/java/org/zanata/util/HashUtil.java 73.68% <0%> (-4.1%) 7% <0%> (ø)
...va/org/zanata/util/SynchronizationInterceptor.java 68.42% <0%> (-3.81%) 3% <0%> (ø)
...ta-model/src/main/java/org/zanata/tmx/TMXUtil.java 82.5% <0%> (-2.12%) 17% <0%> (ø)
...va/org/zanata/search/AbstractIndexingStrategy.java 56.66% <0%> (-1.96%) 5% <0%> (ø)
.../main/java/org/zanata/util/CommonMarkRenderer.java 76.19% <0%> (-1.86%) 6% <0%> (ø)
...zanata/search/HTextFlowTargetIndexingStrategy.java 68.29% <0%> (-1.71%) 7% <0%> (ø)
...ontend/src/app/editor/reducers/settings-reducer.js 90.19% <0%> (-1.48%) 0% <0%> (ø)
.../java/org/zanata/rest/client/TraceDebugFilter.java 77.77% <0%> (-1.47%) 12% <0%> (ø)
... and 1665 more

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 9ee37ef...f59430d. Read the comment docs.

@efloden
Copy link
Member Author

efloden commented Jan 22, 2018

@aeng build passing now

@spathare21 spathare21 added Verified and removed On QA labels Jan 22, 2018
@alex-sl-eng
Copy link
Member

@efloden probably its a good idea to increase the test coverage for this component.

@alex-sl-eng
Copy link
Member

:lgtm:


Reviewed 2 of 2 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@efloden efloden merged commit 679c472 into master Feb 2, 2018
@efloden efloden deleted the efloden/refactor-highlightjs-imports branch February 2, 2018 03:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants