Skip to content

Tweak the publication condition to prevent publishing from forked repos#810

Open
danicheg wants to merge 8 commits into
typelevel:mainfrom
danicheg:tweak-publish-step-ci
Open

Tweak the publication condition to prevent publishing from forked repos#810
danicheg wants to merge 8 commits into
typelevel:mainfrom
danicheg:tweak-publish-step-ci

Conversation

@danicheg

@danicheg danicheg commented Apr 6, 2025

Copy link
Copy Markdown
Member

One might argue that users can use githubWorkflowPublishCond to prevent publishing from forked repos. Sure, but I’m confident this condition should be the default. Adding it manually to the build of dozens of projects isn’t exactly ideal, either.
This is basically the same as what we did in #720, but adding the same SBT setting again just feels a bit extra to me 🤷🏻

@danicheg

danicheg commented Apr 6, 2025

Copy link
Copy Markdown
Member Author

I was just thinking again about adding a dedicated SBT setting for the fork condition. Maybe introducing githubWorkflowForkCondition with a default value of github.event.repository.fork == false isn't such a bad idea — especially if we deprecate the tlCiForkCondition and substitute its default with the newly added githubWorkflowForkCondition. WDYT?

@bpholt

bpholt commented Apr 14, 2025

Copy link
Copy Markdown
Member

I think it is a good idea not to publish from forks by default, but I can envision situations where users legitimately would want to do so, so I don't think it should be disabled without a way to re-enable it. A dedicated sbt setting seems reasonable to me.

@armanbilge armanbilge left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, this is a really good change. Sorry it took me so long to review. I have just a couple nitpicks.

Comment thread .gitignore Outdated
Comment thread ci/src/main/scala/org/typelevel/sbt/TypelevelCiPlugin.scala Outdated
Comment thread docs/gha.md Outdated
Comment thread github-actions/src/main/scala/org/typelevel/sbt/gha/GenerativeKeys.scala Outdated
@danicheg danicheg requested a review from armanbilge May 20, 2025 16:47
@danicheg

Copy link
Copy Markdown
Member Author

Any chance we'd merge this?

@armanbilge armanbilge closed this Sep 17, 2025
@armanbilge armanbilge reopened this Sep 17, 2025

@armanbilge armanbilge left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm so sorry—I just realize one more important bikeshed I think.

Currently, githubWorkflowForkCondition == true if the repository is not a fork, which is confusing. (This was the original mistake of tlCiForkCondition).

So either we should call it githubWorkflowNotForkCondition or change the definition to test == true, and then adjust the usecases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants