SF-3882 Remove SMT Translation Suggestions - #4025
Conversation
GitHub Actions
Inactive
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #4025 +/- ##
==========================================
- Coverage 81.06% 80.66% -0.40%
==========================================
Files 660 653 -7
Lines 43036 41515 -1521
Branches 7057 6824 -233
==========================================
- Hits 34887 33488 -1399
+ Misses 6977 6874 -103
+ Partials 1172 1153 -19 ☔ View full report in Codecov by Harness. |
|
📸 Screenshot diff deployed! (3 changes) View the visual diff at: https://pr-4025--sf-screenshot-diffs.netlify.app |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
GitHub Actions
Inactive
pmachapman
changed the title
pmachapman
added
the
will require testing
Nateowami
left a comment
Nateowami
left a comment
There was a problem hiding this comment.
@Nateowami made 1 comment.
Reviewable status: 0 of 94 files reviewed, all discussions resolved.
src/RealtimeServer/scriptureforge/models/sf-project-user-config-test-data.ts line 13 at r2 (raw file):
confidenceThreshold: 0.2, transliterateBiblicalTerms: false, translationSuggestionsEnabled: false,
I'm surprised to see this property remains, rather than being migrated off.
|
(I guess I should state my actual question, which is whether this was a deliberate decision and why) |
pmachapman
left a comment
pmachapman
left a comment
There was a problem hiding this comment.
@pmachapman made 1 comment.
Reviewable status: 0 of 94 files reviewed, all discussions resolved (waiting on Nateowami).
src/RealtimeServer/scriptureforge/models/sf-project-user-config-test-data.ts line 13 at r2 (raw file):
Previously, Nateowami wrote…
I'm surprised to see this property remains, rather than being migrated off.
If we remove the property outright in this PR and deploy to live, anyone with the older code base will cause exceptions as the missing property is attempted to be read and undefined is returned.
I've take the approach of removing references to it in the frontend and backend code, and marking it as obsolete. We will be able to remove it sometime in the future (maybe in 4-6 months time?) and very few if any exceptions will result, as nearly all users will be using a codebase that doesn't reference the property.
pmachapman
added
the
do not merge
GitHub Actions
Inactive
3 participants
This change is