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

Migrate Ores calls to Liftwing #21

Merged
merged 4 commits into from Oct 19, 2023
Merged

Migrate Ores calls to Liftwing #21

merged 4 commits into from Oct 19, 2023

Conversation

chukarave
Copy link
Collaborator

@chukarave chukarave commented Sep 14, 2023

Implement score calculation using Liftwing instead of ORES. This required multiple changes to the ArticleQualityService as Liftwing calls are built differently.

Bug: T343731

Copy link
Member

@itamargiv itamargiv left a comment

Choose a reason for hiding this comment

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

There's more work to be done. I've added a few comments for this round. As a reminder to our takeaways from Friday, please remove the following from the commit:

  • All the changes made to components
  • All the changes made in the tests.

As for the tests, I think we shall rewrite them in the end. But as a first step, I think we should remove the tests for getOresScores and all the mocks it requires. I think it'll be best to just start from a clean slate when starting to write tests from the methods added in this commit.

src/ArticleQualityService.ts Outdated Show resolved Hide resolved
src/ArticleQualityService.ts Show resolved Hide resolved
src/ArticleQualityService.ts Show resolved Hide resolved
src/ArticleQualityService.types.ts Outdated Show resolved Hide resolved
@chukarave
Copy link
Collaborator Author

chukarave commented Sep 26, 2023

@itamargiv What's the wisdom about the tests? I removed Ores related mocks and tests, no other changes yet. my current blocker is still failing to make more than one fetch.mock() call in a row: fetch-mock: No fallback response defined for POST

@chukarave chukarave marked this pull request as ready for review October 17, 2023 09:12
chukarave and others added 4 commits October 19, 2023 10:41
- Adjust scores objects to liftwing response structure
- Provide scores from liftwing
- Move iteration out of provideScores and clean non-liftwing related stuff from pr
- Use jest-fetch-mock and fix tests
This was done to allow us to use ignore rule temporarly until we
come around to finish updating IQE's dependencies.
@itamargiv itamargiv merged commit 9887233 into main Oct 19, 2023
4 checks passed
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.

None yet

2 participants