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

Add required phrase rules automatically #3254

Closed
wants to merge 15 commits into from
Closed

Add required phrase rules automatically #3254

wants to merge 15 commits into from

Conversation

AyanSinhaMahapatra
Copy link
Member

@AyanSinhaMahapatra AyanSinhaMahapatra commented Feb 15, 2023 •
edited
Loading

Reference: #2637 #2878

Tasks

  • Reviewed contribution guidelines
  • PR is descriptively titled 📑 and links the original issue above 🔗
  • Tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR
    Run tests locally to check for errors.
  • Commits are in uniquely-named feature branch and has no merge conflicts 📁

@AyanSinhaMahapatra
Add script and functions for adding required phrases
9a995dd 
Add a script which can add required phrases in already existing rules
automatically from license field names, and can do this one license
expression at a time.

Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Add required phrases for apache-2.0 rules
8cb1fe4 
These were added automatically by the script added at
etc/scripts/licenses/add_required_phrases.py

Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Add required phrases for apache-1.1 rules
0da1c0b 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Update test expectaion
df5ddd7 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra AyanSinhaMahapatra changed the title Required phrase rules Add required phrase rules automatically Feb 17, 2023
@AyanSinhaMahapatra AyanSinhaMahapatra mentioned this pull request Mar 26, 2023
Open
@pombredanne pombredanne marked this pull request as draft April 24, 2023 15:45
@AyanSinhaMahapatra AyanSinhaMahapatra mentioned this pull request Aug 27, 2023
Closed
@AyanSinhaMahapatra AyanSinhaMahapatra mentioned this pull request Sep 15, 2023
Closed
@AyanSinhaMahapatra
Merge branch 'develop' into required-phrase-rules
48370a5 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra AyanSinhaMahapatra added this to the v32.1 milestone Jan 15, 2024
@AyanSinhaMahapatra
Merge branch 'develop' into required-phrase-rules
27f2d1c 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Move required-phrases script to licensedcode
9e49d80 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Add a new console script to add required phrases
9892ec2 
* moves the script to licensedcode
* adds a new console script `add-required-phrases`
* move the functions and tests to seperate files

Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Update apache-2.0 rules with required phrases
109407a 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra AyanSinhaMahapatra marked this pull request as ready for review January 29, 2024 10:37
@AyanSinhaMahapatra AyanSinhaMahapatra modified the milestones: v32.1, v32.2 Feb 19, 2024
@pombredanne pombredanne mentioned this pull request Mar 20, 2024
Open
@pombredanne pombredanne mentioned this pull request Apr 12, 2024
Open
@AyanSinhaMahapatra AyanSinhaMahapatra mentioned this pull request Apr 26, 2024
Merged
3 tasks
pombredanne
pombredanne reviewed Aug 19, 2024
View reviewed changes
Copy link
Member

@pombredanne pombredanne left a comment

Choose a reason for hiding this comment

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

Here are some comments for your consideration.

src/licensedcode/models.py Outdated Show resolved Hide resolved
src/licensedcode/models.py Outdated Show resolved Hide resolved
src/licensedcode/models.py Outdated Show resolved Hide resolved
src/licensedcode/models.py Outdated Show resolved Hide resolved
src/licensedcode/models.py Outdated Show resolved Hide resolved
src/licensedcode/models.py Show resolved Hide resolved
src/licensedcode/models.py Outdated Show resolved Hide resolved
src/licensedcode/tokenize.py Outdated Show resolved Hide resolved
@AyanSinhaMahapatra
Add required phrases from other rules
f13b100 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Update and improve collecting and marking required phrases
8241cee 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Add changes on marking required phrases automatically
6d93bba 
This is done only on `mit` rules, by running:
add-required-phrases -ovr --license-expression "mit"

Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
pombredanne
pombredanne reviewed Aug 29, 2024
View reviewed changes
Copy link
Member

@pombredanne pombredanne left a comment

Choose a reason for hiding this comment

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

Some minor nits and a question on the is_required_phrase flag for your consideration!

src/licensedcode/tokenize.py Outdated Show resolved Hide resolved
src/licensedcode/tokenize.py Outdated Show resolved Hide resolved
src/licensedcode/tokenize.py Outdated Show resolved Hide resolved
src/licensedcode/models.py Outdated Show resolved Hide resolved
src/licensedcode/models.py Show resolved Hide resolved
src/licensedcode/models.py Outdated Show resolved Hide resolved
src/licensedcode/models.py Show resolved Hide resolved
src/licensedcode/models.py Outdated Show resolved Hide resolved
src/licensedcode/models.py
'but is an essential section of the rule text which must be '
'present in the case of partial matches, otherwise the match '
'will be a false positive and misleading.')
)
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this would not be best computed an index time ... this is True if a rule text is entirely a required phrase, this would mean one less stored flag.

src/licensedcode/models.py Show resolved Hide resolved
@AyanSinhaMahapatra
Fix test failures and address feedback
04ae85d 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
@AyanSinhaMahapatra
Fix required phrase span bugs
0eef6c5 
Signed-off-by: Ayan Sinha Mahapatra <ayansmahapatra@gmail.com>
pombredanne
pombredanne approved these changes Sep 2, 2024
View reviewed changes
Copy link
Member

@pombredanne pombredanne left a comment

Choose a reason for hiding this comment

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

Looking fine!

This was referenced Sep 10, 2024
Open
Open
@AyanSinhaMahapatra AyanSinhaMahapatra mentioned this pull request Sep 18, 2024
Open
4 tasks
@AyanSinhaMahapatra
Copy link
Member Author

Closing this as we now have #3924

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pombredanne pombredanne pombredanne approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v32.2
Development

Successfully merging this pull request may close these issues.
2 participants
@AyanSinhaMahapatra @pombredanne