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

ci: set ".github/**" at paths-ignore #2876

Closed
wants to merge 1 commit into from
Closed

Conversation

EdamAme-x
Copy link
Contributor

@EdamAme-x EdamAme-x commented May 31, 2024

I don't think “.github” needs "ci" of actions either.
#2850

@exoego
Copy link
Contributor

exoego commented May 31, 2024

I am against this change.

.github/workflows contains CI job definitions.
CI should run when CI job definitions get updated (e.g. #2851).

@EdamAme-x
Copy link
Contributor Author

EdamAme-x commented May 31, 2024

I am against this change.

.github/workflows contains CI job definitions. CI should run when CI job definitions get updated (e.g. #2851).

Thank for your reply.

Sorry, I apologize for my lack of knowledge.
I think the CI after the changes are made is executed after the commit (merged), and there is no problem if it is not executed at the time of the PR, isn't it?

We can also ignore other than .github/workflows. What do you think?

@EdamAme-x EdamAme-x marked this pull request as draft May 31, 2024 04:58
@exoego
Copy link
Contributor

exoego commented May 31, 2024

CI after the changes are made is executed after the commit (merged), and there is no problem if it is not executed at the time of the PR, isn't it?

No, it is a problem.
Consider a PR that adds a new version of runtime (e.g. Node.js 24 or Bun2) that introduces breaking changes to Hono.
If CI is skipped, we can not be aware of such breaking changes, and the PR might be merged.
CI on the PRs created after the PR will fail due to the missed breaking changes.
I think it is terrible for contributors/maintainers, because breaking changes often requires time to fix.
We can revert the change easily, but it bothers maintainers.
So I don't believe "CI on main branch is enough" is a good excuse.

We can also ignore other than .github/workflows. What do you think?

I personally think such "smart" configuration should be avoided.
.github can contain many types of files and GitHub may add more that might be related to CI in future.
Maintainers/contributors might get surprised when they update something in .github but CI is skipped.


I personally think "safety" (error prevention) should be prioritized.
So paths-ignore should be used very carefully when:

  • CI is very very slow
  • CI consumes too much money

IMHO, these appear not to apply to the Hono repo right now: the slowest CI job succeeds in about 1m, which is acceptable speed.
image

@EdamAme-x
Copy link
Contributor Author

@exoego thank for your reply!
I understood. I'll close this

@EdamAme-x EdamAme-x closed this May 31, 2024
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