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

Ignore PDF files for packages #1649

Closed
wants to merge 1 commit into from
Closed

Ignore PDF files for packages #1649

wants to merge 1 commit into from

Conversation

Bi0T1N
Copy link
Contributor

@Bi0T1N Bi0T1N commented Jan 30, 2025

This adds an entry to the .gitignore file to ignore all (new) PDF files for packages. Avoids that every single package maintainer may have it's own .gitignore file to achieve the same result... 😄

find packages/ -name .gitignore | wc -l 
154

Related to #1595 (comment)

@elegaanz
Copy link
Member

Thank you, but unfortunately there are a few exceptions to this rule: for instance we generally allow manual files as PDF, especially if they are linked in the README. This .gitignore rule would be too generic to allow for that, and I don't think that you can refine it to allow only for some specific PDF files (judging the contents of the PDF can probably only be done by a human). I'll close for this reason.

@elegaanz elegaanz closed this Jan 31, 2025
@Bi0T1N
Copy link
Contributor Author

Bi0T1N commented Jan 31, 2025

Well, even if the .gitignore has a line to skip PDF files for packages you can add them to the commit by using git add --force wanted.pdf.
The other option I see to make it easier for maintainers is to allow the .gitignore file within the package directory. Currently the pipeline fails if you do so:

Error: packages/preview/iconic-salmon-svg/.gitignore: a package directory may only contain version sub-directories, not files.

The current state is that you would need to have a single .gitignore file in every version sub-directory which is cumbersome and prone to errors. And somehow useless, because you no longer touch the version directory after the merge to the main branch.

@Bi0T1N
Copy link
Contributor Author

Bi0T1N commented Jan 31, 2025

Should I create an issue for this if further discussion is needed?

@elegaanz
Copy link
Member

elegaanz commented Feb 3, 2025

Yes please, it will be more visible than this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants