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

Add page to configure draft sources #2962

Draft
wants to merge 49 commits into
base: master
Choose a base branch
from

Conversation

Nateowami
Copy link
Collaborator

@Nateowami Nateowami commented Jan 18, 2025

This change is Reviewable

@Nateowami Nateowami added the will require testing PR should not be merged until testers confirm testing is complete label Jan 18, 2025
@marksvc marksvc force-pushed the feature/configure-sources branch from 615d115 to 0bf8bbd Compare January 20, 2025 19:43
@Nateowami
Copy link
Collaborator Author

@marksvc Could you resolve merge conflicts, since they're on changes you made?

@marksvc marksvc force-pushed the feature/configure-sources branch from d185c4b to db36b13 Compare February 5, 2025 18:51
@marksvc
Copy link
Collaborator

marksvc commented Feb 5, 2025

@Nateowami You might already have a great conflict resolution tool. If not, FYI that kdiff3 automatically solves a number of conflicts that git doesn't already automatically solve. kdiff3's interface is a learning curve, but it's worth it once you get comfortable with the tool.

I still had to think about what was going on in the remaining conflicts (and it makes sense that you asked me to resolve it since I worked on it), but it is a nice way to address conflicts. And a good time to mention the useful tool :)

image

@Nateowami Nateowami force-pushed the feature/configure-sources branch from db36b13 to 9e4e28a Compare February 6, 2025 23:30
@Nateowami Nateowami force-pushed the feature/configure-sources branch from 57f317a to 7bc01ef Compare February 13, 2025 00:57
@marksvc marksvc force-pushed the feature/configure-sources branch from 7bc01ef to 0883547 Compare February 18, 2025 22:47
@marksvc
Copy link
Collaborator

marksvc commented Feb 18, 2025

(I pushed a conflict resolution.)

Copy link

codecov bot commented Feb 18, 2025

Codecov Report

Attention: Patch coverage is 53.15789% with 89 lines in your changes missing coverage. Please review.

Project coverage is 82.47%. Comparing base (46214a7) to head (6b9dbb9).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
...eneration/draft-sources/draft-sources.component.ts 46.29% 75 Missing and 12 partials ⚠️
...ripture/ClientApp/src/app/core/paratext.service.ts 0.00% 1 Missing ⚠️
.../src/app/translate/draft-generation/draft-utils.ts 95.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2962      +/-   ##
==========================================
- Coverage   82.64%   82.47%   -0.17%     
==========================================
  Files         553      554       +1     
  Lines       32010    32197     +187     
  Branches     5164     5230      +66     
==========================================
+ Hits        26454    26554     +100     
- Misses       4797     4872      +75     
- Partials      759      771      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@marksvc marksvc force-pushed the feature/configure-sources branch from feaf535 to e0f85ff Compare February 19, 2025 22:57
@marksvc
Copy link
Collaborator

marksvc commented Feb 19, 2025

I worked on designing a TestActivatedProjectModule that we could import to help us have needed dependencies for TestActivatedProjecctService. Unfortunately I didn't really have a working base upon which to make the change and work toward the tests working again.

After working on it for a bit, I had tests running more nicely, but failing, presumably without project data being provided in the right places.

So I rolled back and wrote a simpler solution to TestActivatedProjectService. After working on that for a bit, I had all the same problems as before.

I committed both. We need to provide the list of projects to the tests.

@marksvc marksvc force-pushed the feature/configure-sources branch from e0f85ff to 6fbf7ff Compare February 20, 2025 23:00
marksvc and others added 5 commits February 21, 2025 10:41
Based on some discussion of the philosophy of testing this component
at this time.

The 'should not save when language codes are not confirmed' test would
be presumably passing for the wrong reason at this time.
…hat are no longer available
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
will require testing PR should not be merged until testers confirm testing is complete
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants