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.

Sign up for GitHub

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

Jump to bottom

feat: Adding Knowledge Panel for Last-edits #13

Closed
wants to merge 18 commits into from
Closed

feat: Adding Knowledge Panel for Last-edits #13

wants to merge 18 commits into from

Conversation

sumit-158
Copy link
Member

@sumit-158 sumit-158 commented Jul 19, 2022 •
edited
Loading

What

Fixes bug(s)

@sumit-158 sumit-158 linked an issue Jul 19, 2022 that may be closed by this pull request
Last edits as knowledge panel #12
Closed
@sumit-158 sumit-158 marked this pull request as ready for review July 19, 2022 11:39
@sumit-158 sumit-158 requested review from alexgarel and teolemon July 19, 2022 11:39
@sumit-158 sumit-158 changed the title Adding Knowledge Panel for Last-edits feat: Adding Knowledge Panel for Last-edits Jul 20, 2022
teolemon
teolemon reviewed Jul 20, 2022
View reviewed changes
app/knowledge_panels.py Outdated Show resolved Hide resolved
teolemon
teolemon reviewed Jul 20, 2022
View reviewed changes
app/knowledge_panels.py Outdated Show resolved Hide resolved
teolemon
teolemon reviewed Jul 20, 2022
View reviewed changes
app/knowledge_panels.py Outdated Show resolved Hide resolved
teolemon
teolemon reviewed Jul 20, 2022
View reviewed changes
app/main.py Outdated Show resolved Hide resolved
teolemon
teolemon reviewed Jul 20, 2022
View reviewed changes
app/main.py Outdated Show resolved Hide resolved
alexgarel
alexgarel reviewed Jul 20, 2022
View reviewed changes
app/knowledge_panels.py Outdated Show resolved Hide resolved
sumit-158 and others added 3 commits July 22, 2022 18:51
@sumit-158
Update knowledge_panels.py
2750afe
@sumit-158 @teolemon
Update app/main.py
fd7fa8a 
Co-authored-by: Pierre Slamich <pierre.slamich@gmail.com>
@sumit-158 @teolemon
Update app/main.py
9e6c17c 
Co-authored-by: Pierre Slamich <pierre.slamich@gmail.com>
alexgarel
alexgarel reviewed Jul 23, 2022
View reviewed changes
app/knowledge_panels.py Outdated Show resolved Hide resolved
alexgarel
alexgarel reviewed Jul 23, 2022
View reviewed changes
app/knowledge_panels.py Outdated Show resolved Hide resolved
app/knowledge_panels.py Outdated Show resolved Hide resolved
app/knowledge_panels.py Outdated Show resolved Hide resolved
alexgarel
alexgarel reviewed Jul 26, 2022
View reviewed changes
Copy link
Member

@alexgarel alexgarel left a comment

Choose a reason for hiding this comment

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

We are on the right path :-)

app/knowledge_panels.py Outdated Show resolved Hide resolved
app/knowledge_panels.py Outdated Show resolved Hide resolved
app/knowledge_panels.py Outdated Show resolved Hide resolved
app/knowledge_panels.py Outdated Show resolved Hide resolved
app/knowledge_panels.py Outdated Show resolved Hide resolved
tests/test_main.py Outdated Show resolved Hide resolved
@sumit-158 sumit-158 marked this pull request as draft July 26, 2022 10:44
teolemon
teolemon reviewed Jul 27, 2022
View reviewed changes
app/models.py Outdated
Comment on lines 47 to 50
if facet == "packaging":
facet_plural = facet
else:
facet_plural = plural
Copy link
Member

Choose a reason for hiding this comment

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

Why do we handle differently for packaging ?

Suggested change
if facet == "packaging":
facet_plural = facet
else:
facet_plural = plural
facet_plural = plural
# special case for packaging ?
if facet == "packaging":
facet_plural = facet

Copy link
Member Author

Choose a reason for hiding this comment

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

because that how the search api takes it

teolemon
teolemon reviewed Jul 27, 2022
View reviewed changes
app/knowledge_panels.py Outdated Show resolved Hide resolved
teolemon
teolemon reviewed Jul 27, 2022
View reviewed changes
app/knowledge_panels.py Outdated Show resolved Hide resolved
teolemon
teolemon reviewed Jul 27, 2022
View reviewed changes
app/off.py Show resolved Hide resolved
alexgarel
alexgarel reviewed Jul 27, 2022
View reviewed changes
app/off.py Show resolved Hide resolved
sumit-158
sumit-158 commented Jul 28, 2022
View reviewed changes
tests/test_utils.py Outdated Show resolved Hide resolved
alexgarel
alexgarel reviewed Jul 28, 2022
View reviewed changes
Copy link
Member

@alexgarel alexgarel left a comment

Choose a reason for hiding this comment

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

A little help with monkey patching :-)

tests/test_utils.py Outdated Show resolved Hide resolved
tests/test_utils.py Outdated Show resolved Hide resolved
tests/test_knowledge_panels.py Show resolved Hide resolved
tests/test_knowledge_panels.py Outdated Show resolved Hide resolved
@sumit-158 sumit-158 marked this pull request as ready for review July 29, 2022 14:57
@sumit-158
Copy link
Member Author

sumit-158 commented Jul 29, 2022 •
edited
Loading

Closing this issue started fresh pr #16 :)

@sumit-158 sumit-158 closed this Jul 29, 2022
@sumit-158 sumit-158 deleted the kp_last_edits branch July 29, 2022 15:39
@teolemon teolemon mentioned this pull request Aug 28, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@teolemon teolemon teolemon left review comments

@alexgarel alexgarel alexgarel left review comments

Assignees

@sumit-158 sumit-158

Labels
None yet
Projects
Knowledge Panels for Facets
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Last edits as knowledge panel
3 participants
@sumit-158 @alexgarel @teolemon