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.

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

convert fails conda-smithy linter #280

Closed
lucascolley opened this issue Dec 31, 2024 · 8 comments · Fixed by #283
Closed

convert fails conda-smithy linter #280

lucascolley opened this issue Dec 31, 2024 · 8 comments · Fixed by #283
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@lucascolley
Copy link

lucascolley commented Dec 31, 2024

What happened?
The following diff was needed to pass the conda-smithy linter after using convert:

diff --git a/recipe/recipe.yaml b/recipe/recipe.yaml
index 372e4c3..26fe8d2 100644
--- a/recipe/recipe.yaml
+++ b/recipe/recipe.yaml
@@ -1,5 +1,3 @@
-schema_version: 1
-
 context:
   name: array-api-extra
   version: 0.4.0
@@ -56,4 +54,3 @@ about:
 extra:
   recipe-maintainers:
     - lucascolley
-

To Reproduce

Steps to reproduce the behavior:

Additional Details (please complete the following information):
version 0.4.0

@lucascolley lucascolley added bug Something isn't working to sort Needs additional investigation labels Dec 31, 2024
@schuylermartin45
Copy link
Collaborator

schema_version is a required field for all V1 recipe files. It is a future-proofing component of the spec. If this linter is having an issue with that, it is a problem with the linter and not Conda Recipe Manager.

As for the trailing whitespace, I don't think this is the fault of the CRM project. In other words, CRM did nothing wrong, this is a tabs-vs-spaces-esk styling debate. We are not beholden to the preferences of conda-forge, we aim to provide tooling that can be used by the conda community as a whole. In other repositories, this is the norm.

We could add a flag to control the trailing whitespace. Honestly if this is being run in an automated environment, I would just run a sed/tr command to handle that. I'll leave this ticket open for discussion on that point.

@schuylermartin45 schuylermartin45 added good first issue Good for newcomers and removed to sort Needs additional investigation labels Jan 2, 2025
@lucascolley
Copy link
Author

thanks, opened conda-forge/conda-smithy#2201

@mgorny
Copy link

mgorny commented Jan 6, 2025

As for the trailing whitespace, I don't think this is the fault of the CRM project. In other words, CRM did nothing wrong, this is a tabs-vs-spaces-esk styling debate.

I'm sorry if I'm missing something but to the best of my knowledge, the debate is about having trailing newline and not having a trailing newline. However, conda-recipe-manager generates two trailing newlines (i.e. \n\n) which is something I haven't ever seen, short of an actual templating bug.

@schuylermartin45
Copy link
Collaborator

Two newlines? I only see one blank line at the end of the diff provided. One blank line is also all I have ever seen come out of this tool and what our CRM unit tests have.

Maybe the diff provided is incorrect? Am I missing something?

@lucascolley
Copy link
Author

@schuylermartin45
Copy link
Collaborator

Okay, I'm getting tripped up on the visualization here and had to sanity checked myself with xxd. So I'm wrong, 1 blank line at the end of the file is two new line characters.

Anyways, I'm just going to change it. I could have sworn this was the preferred style in AnacondaRecipes and/or Bioconda but I'm looking again and I don't see a huge preference. It probably just isn't as strictly enforced.

@schuylermartin45 schuylermartin45 self-assigned this Jan 6, 2025
@schuylermartin45 schuylermartin45 linked a pull request Jan 6, 2025 that will close this issue
@github-project-automation github-project-automation bot moved this from Blocked to Done in Conda Recipe Manager Jan 6, 2025
@schuylermartin45
Copy link
Collaborator

Dealt with in: #283 will be in the next release

@lucascolley
Copy link
Author

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants