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

Handle new TSV filename style; other small fixes #155

Merged
merged 7 commits into from
Apr 7, 2021
Merged

Conversation

RobH123
Copy link
Contributor

@RobH123 RobH123 commented Mar 3, 2021

This change is Reviewable

@RobH123 RobH123 linked an issue Mar 3, 2021 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Mar 3, 2021

Codecov Report

Merging #155 (5268098) into master (05e01f5) will decrease coverage by 10.44%.
The diff coverage is 45.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #155       +/-   ##
===========================================
- Coverage   46.71%   36.26%   -10.45%     
===========================================
  Files          26       27        +1     
  Lines        4044     4059       +15     
  Branches     1312     1314        +2     
===========================================
- Hits         1889     1472      -417     
- Misses       1369     1655      +286     
- Partials      786      932      +146     
Impacted Files Coverage Δ
src/core/BCS-usfm-grammar-check.js 54.66% <ø> (ø)
src/core/field-link-check.js 0.00% <0.00%> (ø)
src/core/getApi.js 10.65% <0.00%> (-0.07%) ⬇️
src/core/notes-tsv7-row-check.js 2.70% <0.00%> (-56.58%) ⬇️
src/core/questions-tsv7-row-check.js 2.61% <0.00%> (-63.70%) ⬇️
src/core/text-handling-functions.js 92.59% <ø> (-0.27%) ⬇️
src/core/tn-tsv9-row-check.js 71.49% <ø> (ø)
src/core/tn-tsv9-table-check.js 65.67% <ø> (ø)
src/core/twl-tsv6-row-check.js 2.52% <0.00%> (ø)
src/core/yaml-text-check.js 1.35% <0.00%> (-0.02%) ⬇️
... and 22 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 05e01f5...5268098. Read the comment docs.

@RobH123 RobH123 marked this pull request as draft March 5, 2021 01:35
@RobH123 RobH123 changed the title Improve notice wording for issue #154 Handle new TSV filename style; other small fixes Mar 5, 2021
@RobH123 RobH123 marked this pull request as ready for review March 22, 2021 02:23
@RobH123 RobH123 requested a review from mandolyte March 22, 2021 02:24
@birchamp birchamp added this to the Sprint 26 milestone Mar 24, 2021
@BincyJ
Copy link

BincyJ commented Mar 31, 2021

@mandolyte, @birchamp would you know what main issue we can connect this PR to?

@BincyJ
Copy link

BincyJ commented Mar 31, 2021

This is a 2.0.0 issue

@BincyJ
Copy link

BincyJ commented Mar 31, 2021

@birchamp can we take the 1.2 tag from this PR?

@RobH123 RobH123 linked an issue Apr 6, 2021 that may be closed by this pull request
@RobH123
Copy link
Contributor Author

RobH123 commented Apr 6, 2021

@mandolyte Could you please look at this PR before I start on the next round of content publishing. Thanks.

Copy link
Contributor

@mandolyte mandolyte left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 6 files at r1, 35 of 35 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @RobH123)


src/core/defaults.js, line 7 at r2 (raw file):

  'LT', 'ST',
  'TA', 'TW', 'TWL',
  'TN', 'TN2', 'TQ', 'TQ2',

are still going to need the "...2" repo codes now?

@mandolyte mandolyte merged commit 17d7626 into master Apr 7, 2021
@mandolyte mandolyte deleted the RJHimprovements branch April 7, 2021 11:53
@RobH123
Copy link
Contributor Author

RobH123 commented Apr 8, 2021

@mandolyte We still need to distinguish between the old and new TN and TQ formats for validation I think. Maybe for the longer term, TN should be TN1, then TN2 could be just TN, and similarly for TQ? It's going to be tricky, I think, over the next year or so as things change around, and I still expect many changes to this CV package as various tool and content migration decisions are made one by one.

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.

Handle new TSV filename style Improve notices like "Unable to find TA link"
4 participants