Skip to content

Conversation

@landonreed
Copy link
Contributor

@landonreed landonreed commented Mar 16, 2020

Checklist

  • Appropriate branch selected (all PRs must first be merged to dev before they can be merged to master)
  • Any modified or new methods or classes have helpful JSDoc and code is thoroughly commented
  • The description lists all applicable issues this PR seeks to resolve
  • The description lists any configuration setting(s) that differ from the default settings
  • All tests and CI builds passing

Description

MTC enhancements re #544.

  • Reverse version dropdown sort
  • Do not allow selecting of merged feeds in merge feeds dropdown
  • Add retrieval method icon to dropdown items/version summary

@landonreed landonreed requested a review from a team as a code owner March 16, 2020 15:06
Copy link
Contributor

@evansiroky evansiroky left a comment

Choose a reason for hiding this comment

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

Just a few minor comments.

@evansiroky evansiroky removed their assignment Mar 19, 2020
}
}

/* eslint-disable complexity */
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove /* eslint-disable complexity */ now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if this is needed anywhere anymore. When I remove these yarn lint runs fine. @evansiroky can you weigh in on whether these are still needed?

Copy link
Contributor

@binh-dam-ibigroup binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

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

Please see my additional comments.
I also agree with @evansiroky's comments.

@codecov-io
Copy link

codecov-io commented Mar 20, 2020

Codecov Report

Merging #545 into dev will decrease coverage by 28.71%.
The diff coverage is 1.83%.

Impacted file tree graph

@@             Coverage Diff             @@
##              dev     #545       +/-   ##
===========================================
- Coverage   44.64%   15.93%   -28.72%     
===========================================
  Files         314      316        +2     
  Lines       17138    16055     -1083     
  Branches     5235     4892      -343     
===========================================
- Hits         7651     2558     -5093     
- Misses       8273    11513     +3240     
- Partials     1214     1984      +770     
Flag Coverage Δ
#end_to_end_tests ?
#unit_tests 15.93% <1.83%> (-0.07%) ⬇️
Impacted Files Coverage Δ
lib/editor/actions/trip.js 25.00% <0.00%> (-23.79%) ⬇️
lib/manager/actions/versions.js 12.60% <0.00%> (-46.89%) ⬇️
...b/manager/components/version/FeedVersionDetails.js 0.00% <0.00%> (-32.88%) ⬇️
...manager/components/version/FeedVersionNavigator.js 0.00% <0.00%> (-57.41%) ⬇️
...ib/manager/components/version/FeedVersionReport.js 0.00% <0.00%> (-52.18%) ⬇️
...anager/components/version/VersionRetrievalBadge.js 0.00% <0.00%> (ø)
...ager/components/version/VersionSelectorDropdown.js 0.00% <0.00%> (ø)
lib/manager/util/version.js 31.81% <0.00%> (-23.26%) ⬇️
lib/types/index.js 0.00% <ø> (ø)
lib/manager/actions/status.js 21.38% <3.03%> (-39.12%) ⬇️
... and 271 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 a106298...e9a558c. Read the comment docs.

@landonreed
Copy link
Contributor Author

@evansiroky, I have addressed your comments. @binh-dam-ibigroup, I addressed one comment, but skipped the eslint disable one based on our discussion on IBI chat. Also, I'm not keen on changing the prop name unless we did it wholesale throughout other components, which seems a bit out of scope.

Copy link
Contributor

@evansiroky evansiroky left a comment

Choose a reason for hiding this comment

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

Travis builds are failing.

@evansiroky evansiroky assigned landonreed and unassigned evansiroky Mar 26, 2020
@landonreed landonreed assigned evansiroky and unassigned landonreed Mar 26, 2020
@evansiroky evansiroky removed their assignment Mar 26, 2020
@landonreed landonreed changed the title Reverse version sort; add retrieval method filtering MTC Enhancements March 2020 Apr 10, 2020
@codecov-commenter
Copy link

codecov-commenter commented May 27, 2020

Codecov Report

Merging #545 into dev will decrease coverage by 23.03%.
The diff coverage is 6.91%.

Impacted file tree graph

@@             Coverage Diff             @@
##              dev     #545       +/-   ##
===========================================
- Coverage   38.42%   15.38%   -23.04%     
===========================================
  Files         315      323        +8     
  Lines       17255    16797      -458     
  Branches     5268     5175       -93     
===========================================
- Hits         6630     2584     -4046     
- Misses       9246    12066     +2820     
- Partials     1379     2147      +768     
Flag Coverage Δ
#end_to_end_tests ?
#unit_tests 15.38% <6.91%> (-0.44%) ⬇️
Impacted Files Coverage Δ
lib/common/util/config.js 70.90% <0.00%> (-9.10%) ⬇️
lib/editor/actions/trip.js 25.00% <0.00%> (-0.21%) ⬇️
lib/gtfsplus/components/GtfsPlusVersionSummary.js 0.00% <0.00%> (ø)
lib/manager/actions/user.js 16.10% <ø> (-29.74%) ⬇️
lib/manager/actions/versions.js 12.60% <0.00%> (-43.09%) ⬇️
lib/manager/components/FeedSourceSettings.js 0.00% <0.00%> (-53.63%) ⬇️
lib/manager/components/GeneralSettings.js 0.00% <0.00%> (ø)
...manager/components/transform/FeedTransformRules.js 0.00% <0.00%> (ø)
...manager/components/transform/FeedTransformation.js 0.00% <0.00%> (ø)
...components/transform/FeedTransformationSettings.js 0.00% <0.00%> (ø)
... and 283 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 7ed815f...3d2c3c0. Read the comment docs.

@landonreed landonreed mentioned this pull request May 27, 2020
5 tasks
@landonreed landonreed merged commit c3cf9eb into dev Jul 15, 2020
@landonreed landonreed mentioned this pull request Aug 12, 2020
8 tasks
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.

6 participants