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

added thoth-license-solver #2647

Merged
merged 3 commits into from
Aug 10, 2022
Merged

added thoth-license-solver #2647

merged 3 commits into from
Aug 10, 2022

Conversation

meile18
Copy link
Contributor

@meile18 meile18 commented Jun 9, 2022 •
edited
Loading

This introduces a breaking change

  • No

This should yield a new module release

  • Yes

This Pull Request implements

added license-solver

Adds:

  • new table python_package_license
  • new columns in python_package_version for python_package_license

Description

into python_package_version will be saved normalized license namings

@sesheta sesheta added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 9, 2022
@sesheta sesheta requested review from harshad16 and KPostOffice June 9, 2022 08:16
KPostOffice
KPostOffice reviewed Jun 9, 2022
View reviewed changes
Copy link
Member

@KPostOffice KPostOffice left a comment

Choose a reason for hiding this comment

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

This /lgtm , another set of eyes would be nice though

harshad16
harshad16 approved these changes Jun 17, 2022
View reviewed changes
Copy link
Member

@harshad16 harshad16 left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve
thanks 💯

@sesheta sesheta added the lgtm Indicates that a PR is ready to be merged. label Jun 17, 2022
@meile18
Copy link
Contributor Author

meile18 commented Jul 13, 2022

/CC @goern

@sesheta sesheta requested a review from goern July 13, 2022 11:58
@goern
Copy link
Member

goern commented Jul 19, 2022

/approve
💯 nice to see this part move forward!!

@goern
Copy link
Member

goern commented Jul 19, 2022

/assign @mayaCostantini

mayaCostantini
mayaCostantini reviewed Jul 20, 2022
View reviewed changes
Copy link
Contributor

@mayaCostantini mayaCostantini left a comment

Choose a reason for hiding this comment

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

looks really good 💯 Just a few comments
Have you tested those changes locally?

thoth/storages/data/alembic/versions/2a4632197512_added_license_solver.py Outdated Show resolved Hide resolved
thoth/storages/graph/models.py Outdated Show resolved Hide resolved
@mayaCostantini
Copy link
Contributor

ping @meile18 :)

@sesheta sesheta removed the lgtm Indicates that a PR is ready to be merged. label Aug 10, 2022
@sesheta
Copy link
Member

sesheta commented Aug 10, 2022

New changes are detected. LGTM label has been removed.

mayaCostantini
mayaCostantini approved these changes Aug 10, 2022
View reviewed changes
Copy link
Contributor

@mayaCostantini mayaCostantini left a comment

Choose a reason for hiding this comment

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

Great work! 🥇 lgtm, please squash your commits before merging

@sesheta
Copy link
Member

sesheta commented Aug 10, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: goern, harshad16, mayaCostantini

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sesheta sesheta added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 10, 2022
@sesheta sesheta merged commit b606074 into thoth-station:master Aug 10, 2022
@harshad16 harshad16 mentioned this pull request Aug 23, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KPostOffice KPostOffice KPostOffice left review comments

@harshad16 harshad16 harshad16 approved these changes

@mayaCostantini mayaCostantini mayaCostantini approved these changes

@goern goern Awaiting requested review from goern

Assignees

@harshad16 harshad16

@mayaCostantini mayaCostantini

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@meile18 @goern @mayaCostantini @sesheta @harshad16 @KPostOffice