-
Notifications
You must be signed in to change notification settings - Fork 11
Removed MIT as a supported license #212
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
base: main
Are you sure you want to change the base?
Conversation
|
@pysojic welcome and thanks for the PR! Most of this looks fine to me, but there's a couple stray changes as noted that don't seem right. We'll see what others think. |
|
Approved. @JeffGarland -- ignore those apparent stray changes -- this repository includes copies of |
|
Hello! Thanks @pysojic and welcome to Beman! As a context for @bemanproject/project-leads , Pierre is a student and I will mentor him to contribute to Beman. Things might not be perfect or expected from first PRs, I will handle the onboard. TLDR: You can ignore this PR for the moment. CC: @ednolan @JeffGarland |
Agree, we don't need to update these copies. And we definitely don't need to update them in this PR (expect the license related ones). CC: @ednolan @JeffGarland |
neatudarius
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again @pysojic for contributing in Beman!
Your core implementation (inside beman_tidy/lib) it's 100% complete and correct. The updates in the tests/ directory also have changes which are at least out of scope for this PR (and also I think they will decrease our coverage).
Please check my comments:
| ## Other | ||
|
|
||
| This is a valid README.md file that follows the Beman Standard: the license is properly formatted - MIT License. | ||
| This project is licensed under the Apache License 2.0 with LLVM Exceptions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JeffGarland , here is a valid and required update for this PR.
CC: @pysojic , please keep this change.
| This is a valid README.md file that follows the Beman Standard: the library status is properly formatted. | ||
|
|
||
| This is a valid README.md file that follows the Beman Standard: the license is properly formatted - MIT License. | ||
| This is a valid README.md file that follows the Beman Standard: the license is properly formatted. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| This is a valid README.md file that follows the Beman Standard: the license is properly formatted. | |
| This is a valid README.md file that follows the Beman Standard: the license is properly formatted - Apache License. |
Please be consistent with previous wording
| Path(f"{valid_prefix}/README-v1.md"), | ||
| Path(f"{valid_prefix}/README-v2.md"), | ||
| Path(f"{valid_prefix}/README-v3.md"), | ||
| Path(f"{valid_prefix}/README-v4.md"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not delete this. Check the comment for updating README-v4 to be still relevant readme.
| Path(f"{valid_prefix}/README-v1.md"), | ||
| Path(f"{valid_prefix}/README-v2.md"), | ||
| Path(f"{valid_prefix}/README-v3.md"), | ||
| Path(f"{valid_prefix}/README-v4.md"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not delete this. Check the comment for updating README-v4 to be still relevant readme.
| # License: Apache License 2.0 with LLVM Exceptions | ||
| Path(f"{valid_prefix}/README-v3.md"), | ||
| # License: Apache License 2.0 with LLVM Exceptions and MIT License | ||
| Path(f"{valid_prefix}/README-v4.md"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not delete this. Check the comment for updating README-v4 to be still relevant readme.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see a reason to delete this file!
Again, it's used to increase coverage for multiple rules
- license
- badges
- status lines
- implement lines
The removal of files v4 and v5 are out of scope for current issue (MIT license). Please put v5 back
| Path(f"{valid_prefix}/README-v2.md"), | ||
| Path(f"{valid_prefix}/README-v3.md"), | ||
| Path(f"{valid_prefix}/README-v4.md"), | ||
| Path(f"{valid_prefix}/README-v5.md"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly, we should revert this line (keep v5 in tests).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good! Will make the changes you mentioned over the weekend!
Approving -- we'll let you handle it :) Thx for the context |
This pull request removes support for the MIT license from the beman-tidy repository.
Changes included: