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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

add .tap/ to .gitignore #51

Closed
2 tasks done
Uzlopak opened this issue Oct 11, 2023 · 13 comments 路 Fixed by #52
Closed
2 tasks done

add .tap/ to .gitignore #51

Uzlopak opened this issue Oct 11, 2023 · 13 comments 路 Fixed by #52
Assignees

Comments

@Uzlopak
Copy link
Contributor

Uzlopak commented Oct 11, 2023

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the feature has not already been requested

馃殌 Feature Proposal

Add .tap/ to .gitignore. It seems that tap version 18 is generating the artifacts. Thats why @groozin was having "suddenly" the .tap folder issue.

As github-action-merge-dependabot was the first repo which could move to tap 18, it was the first repo, which showed this issue.

We should be prepared and add .tap/ folder to the skeleton and from the skeleton to all repos, as usual.

@Fdawgs
@simoneb

Motivation

No response

Example

No response

@gurgunday
Copy link
Member

In which repo were they having issues?

tap 18 uses the Blue Oak license and it鈥檚 disallowed for the moment - so we won鈥檛 be able to update anyway

I also want to signal the fact that license checker is not picking this up

@gurgunday
Copy link
Member

Of course that doesn鈥檛 mean we shouldn鈥檛 add .tap to gitignore

Just pointing it out

@jsumners
Copy link
Member

We cannot, at this time, update to tap@18. See the note at the bottom of pinojs/pino#1762

@Fdawgs
Copy link
Member

Fdawgs commented Oct 11, 2023

We cannot, at this time, update to tap@18. See the note at the bottom of pinojs/pino#1762

Needs tearing out of https://github.com/fastify/github-action-merge-dependabot then :/

@Fdawgs
Copy link
Member

Fdawgs commented Oct 11, 2023

I also want to signal the fact that license checker is not picking this up

Can you clarify?
Seems to be doing its job here: https://github.com/fastify/fastify-static/actions/runs/6177304982/job/16768155505

@Eomm Eomm added the good first issue Good for newcomers label Oct 11, 2023
@Uzlopak
Copy link
Contributor Author

Uzlopak commented Oct 11, 2023

github-action-merge-dependabot does not have a licencechecker job in its workflows. But we also saw tap PRs in other repos, which were rejected because of not supporting node14. So i wonder if any of those PRs triggered the license check job to fail.

@Fdawgs
Copy link
Member

Fdawgs commented Oct 11, 2023

github-action-merge-dependabot does not have a licencechecker job in its workflows. But we also saw tap PRs in other repos, which were rejected because of not supporting node14. So i wonder if any of those PRs triggered the license check job to fail.

fastify/workflows#97 (comment) :)

@Fdawgs
Copy link
Member

Fdawgs commented Oct 12, 2023

I think we do need to add this to the ignore files.

See fastify/workflows#98 (review)

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Oct 12, 2023

Now that we have feedback by @mcollina , that only productive code needs to have OSI approved licenses, we can progress on this issue.

Sorry for the additional work you had in fastify/workflows#98

@mcollina
Copy link
Member

Let me ask specifically to OpenJS but that's my 2 cents.

@simoneb
Copy link

simoneb commented Oct 12, 2023

Didn't expect that an issue to add a folder to .gitignore would get so much attention 馃槃

@Fdawgs
Copy link
Member

Fdawgs commented Nov 20, 2023

Discussions seem positive as noted in fastify/workflows#97

I'll pick this up and mass update the repos early next year.

@Fdawgs Fdawgs self-assigned this Nov 20, 2023
@Fdawgs Fdawgs removed the good first issue Good for newcomers label Nov 20, 2023
@Fdawgs
Copy link
Member

Fdawgs commented Jan 29, 2024

I will sort this tomorrow now that BlueOak is OSI approved, and then do a batch of PRs to all the repos to update their .gitignores.

Edit: holding fire until fastify/workflows#97 (comment) is resolved.

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 a pull request may close this issue.

7 participants