Linter will now hint if the license_family fields in the about section doesn't exist - #1256
Linter will now hint if the license_family fields in the about section doesn't exist#1256scopatz wants to merge 7 commits into
Conversation
|
See check 12a |
|
12a checks that it is non-empty. This checks that they exist. There are starting to be a lot of recipes without these fields because of grayskull. |
|
Instead of making this a new lint, what about consolidating on 12a? |
|
I'd prefer to wait on more changes to the linting here. We are already trying to get legal advice in some cases ( conda-forge/cfep#23 (comment) ). I know there was some disagreement between folks on licensing in a recipe ( conda-forge/staged-recipes#11010 ). Before making changes to the linter it would be nice to get some clarity on what we would like to do where and form consensus. To be clear this doesn't mean we shouldn't work on this PR, I just think it would be prudent to pause for a bit until this addressed. Perhaps we bring this up in the next core meeting. Thoughts? 🙂 cc @conda-forge/core |
|
@jakirkham - I think that is counter to the goals here. I am hoping to prevent the merger of bad recipes with missing metadata. It is really easy to miss that these fields don't exist, and I have reviewed many PRs in the past few days that don't have them because of grayskull. |
|
If anything this helps put PRs with nebulous metadata on hold until we can figure out what to do with them legally, rather than trying to identify and fix them later in a feedstock |
Co-Authored-By: Isuru Fernando <isuruf@gmail.com>
|
@scopatz, it's not clear to me the issue is actually a technical one, in which case a technical fix may not help. There may be technical issues as well, but they need to be aligned with the goals of the larger group. |
|
Yeah, I think the questions are
I think the answer to both is yes. If we don't have a license declaration how can we know that we are able to distribute the code? If we do have a license declaration, I think it makes sense to request that people place these into categories, so that other tools can bundle according to the license restrictions their organization has more easily. This PR just puts a stop gap on accidentally doing the wrong thing, which is why I feel a sense of urgency about it. |
|
OK, this has been updated and passes CI now! |
|
The issues around licensing (at least recently) have not been technical AFAICT, but social. What are our values as a community? Which ones take precedent? etc. We can certainly make technical fixes, but that doesn't really answer these underlying questions. Does that make any sense? 🙂 |
|
Yes, that makes sense. I understand they are social. I am trying to buy some time to figure those things out with this PR. I do a lot of reviews on staged-recipes and this PR would make my life a lot easier, by removing another three things I have to automatically check for, and prevent personal burnout. This PR in its current form doesn't change the way we are currently opperating. It just reduces the pain of operating that way. Does that make sense? Hopefully that helps clarify it |
|
This is ready for another look! |
|
I don't know... but I will give my personal opinion on that And on top of that, I think that is violating the DRY principle because we are duplicating information regarding the license if someone needs to know the license, the user can read it because the license is available with the Sorry if I am being a pedant here, but I still cannot understand the advantage of it, because it is not a legal thing (since we are specifying the license and the license file) and it may complicate to write recipes and all of that For example |
|
This PR recommends, not requires, |
Sorry, just to clarify, I am not against it. 😁 I just think that is not really necessary because of the points which I highlighted |
|
Pinging this again @conda-forge/core |
There was a problem hiding this comment.
My point here is that this is a very minimal, 2 line change to help us catch issues potential issues, while we do figure out what we want to do with more formal things
I agree with that. But I'd like this PR to be changed to do the opposite, i.e., recommend to remove license_family if present.
Marcelo is right on his assessment and I never understood why this has been there in the first place.
My (very undiplomatic, sorry) stance is that it is
- useless
- not the job (i.e., the categorization of a license) of a packager
- redundant at best
- error-prone
- unnecessarily hard-coded in
conda-build - etc.
|
@mbargull - I am happy with you stance, but someone should write a CFEP for it. We have thousands of packages with this entry right now, and it is still in the example recipe. This is a stop-gap measure to ensure consistency |
@mbargull here is some history on the It was added by AnacondaInc to help costumers easily create package collections that would fit their license needs. At the time I was against enforcing that in conda-forge b/c we don't have those corporate needs but we did let AnacondaInc added them as they please just because that does not hurt our recipes in any way. I still have the same opinion. I don't want to enforce this on the community, but if a corporation needs that field they can send PRs adding them. |
|
Also, I generally agree that the SPDX identifiers are a much better way of doing this kind of thing. However, we have yet to fully adopt those. |
|
I want to point out that @scopatz is doing the vast majority of the heavy lifting on staged-recipes at the moment. Unless some of the others on this thread want to start sharing the load more, I think we should all defer to him on changes that he would like to make his day-to-day easier. That is the original motivation for this change and is unaffected by any of these other, more significant issues. They should be discussed in a CFEP for SPDX identifiers, which I am happy to shepherd myself. |
What is easier then not having a |
|
Deferring to @scopatz on this, for now, is the point, not debating what is easiest for him. |
|
Having this hint would really help me in the short term. I would really appreciate it if this was merged. Otherwise the list of PRs that this issue affects will continue to grow and I will keep having to manually look for it. |
|
@scopatz, can you describe the issue please? Is it just inconsistency? |
|
IMO, just adding/changing a recommendation on such a non-critical part does not need a CFEP. @scopatz, would a hint to remove |
|
Yes, it is inconsistency and we used to have a policy (before policies were as formal as they are now) where we required this field. So I see discussions of getting rid of It is also that if someone did want to filter on license type, right now |
|
Wow. 7k with vs 2k is de facto policy. Stopping adding it without replacement is not a good idea. Again unless any of us plan to do staged recipes reviews at a similar pace as @scopatz, we need to defer to him, not second guess him. |
|
@beckermr, if your concern is just staged-recipes, we can make this a lint only for staged-recipes. |
|
We may not deprecate license_family in the cfep per my concerns that the depreciation would be unfriendly to the people that provide us with free services. My concern here is only that defer to @scopatz and not bike-shed what is a defacto policy right now. A staged recipes only hint would probably be fine but, again, let’s defer to @scopatz. |
@forrestwaters I'd like to know your opinion here. Do you all still rely heavily on the |
|
#1220 is also ready. |
|
(semi off-topic)
That only indicates prevalence. E.g. also only indicates prevalence, but not a policy on how much
We should be critical to ourselves (to a reasonable extent ) and strive improvement ;). @scopatz, it's interesting to me that you think of the removal to be comparable to "breaking an API". Meaning, I hadn't expected such a strong reaction/classification on removing what I thought of as an "annoyance due to historical remnants". |
|
@mbargull @isuruf @jakirkham @marcelotrevisani - yeah, I agree with your sentiments here. They way things are done now, kinda sucks (relying on And @ocefpaf @forrestwaters - Anaconda isn't the only company. There are untold masses that use our packages and their metadata, in large part because they are better than anything else out there in this space. We have to be cognizant of the fact that 90% of them will never communicate with us in any way, and only read the documentation we put out there. For example, consider what would happen if conda-build dropped I think we are all in basic agreement here about what to do, and what the path forward is, so someone should just write the CFEP already 😉 🏆 |
Never said they are the only company. I asked @forrestwaters b/c they are a known company that supports us. |
|
OK, I have fixed the conflicts and this is ready for review again. CC @conda-forge/core |
CJ-Wright
left a comment
There was a problem hiding this comment.
If desired the auto tick bot could help to fill in the license family (where it can inspect one reasonably) for packages that are lacking it.
|
@mbargull - is your change request still valid? |
|
@scopatz, yes, I still think it makes more sense and that it would reduce the amount of review work etc. However, feel free to take that as an advice/recommendation from my part but not as a "decree" -- meaning, I don't insist on blocking this PR! |
|
I'll send a CFEP later today deprecating license_family and using SPDX for |
|
@isuruf We need to wait to hear from @forrestwaters before we deprecate license_family |
|
Sure. Interested parties can weigh in on the CFEP. |
|
The linter now checks for SPDX on license field and the hotfixing code will add a |
Checklist
newsentry