-
Notifications
You must be signed in to change notification settings - Fork 56
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
Add MulanPSL-2.0 support. #127
Conversation
pkg/header/config.go
Outdated
Pattern string `yaml:"pattern"` | ||
SpdxID string `yaml:"spdx-id"` | ||
CopyrightOwner string `yaml:"copyright-owner"` | ||
CopyrightSoftware string `yaml:"copyright-software"` |
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.
Add this field because MulanPSL-2.0 header require a [software] placeholder.
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.
What is the difference between owner and software?
These seem untypical concepts.
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.
What is the difference between owner and software? These seem untypical concepts.
[owner] is the author of the software, and [software] is the name of the software.
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.
Is it the only license having this special definition?
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 searched for a round of license headers and found that only gpl-3.0 header receive an optional software name.
Because the header check
uses FindStringIndex to match, and the software name of the header of the MulanPSL-2.0 protocol is just at the top, so if this placeholder is removed, the header check
should be successful.
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.
Or maybe move the name to the top level?
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.
@kezhenxu94 should be better to provide suggestions on codes. CopyrightSoftware
seems not a very good name.
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.
OK, two things
- Please name this variable as
name
. The name of the project is very clear. I think.
name
is somewhat vague to me because it's under header.license
header: # <1>
license:
name: ... # here
So it may read like "header license's name".
What about prefixing with software or project like softwareName or projectName.
- Document should be updated, https://github.com/apache/skywalking-eyes#configurations.
@kezhenxu94 What do you think? What do you suggest to change the document index? Use
<4>
forname
, and change the followings.
Both is ok. Let's use 4 for name to keep the orders
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.
Or maybe move the name to the top level?
Adding a top-level field is ok in this case but
- this field is only used in header command so we have no idea how to prevent it being used in other places (now or future)
- if the field is used in other places we have no idea how to differentiate them if they have different values by any chance.
- being top-level means it has wider impact and risk of incompatibility when modified
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.
OK, two things
- Please name this variable as
name
. The name of the project is very clear. I think.
name
is somewhat vague to me because it's underheader.license
header: # <1> license: name: ... # hereSo it may read like "header license's name".
What about prefixing with software or project like softwareName or projectName.
- Document should be updated, https://github.com/apache/skywalking-eyes#configurations.
@kezhenxu94 What do you think? What do you suggest to change the document index? Use
<4>
forname
, and change the followings.Both is ok. Let's use 4 for name to keep the orders
I think this is OK
No description provided.