Skip to content

Danger: Fix the config file#33073

Merged
ndelangen merged 7 commits into
nextfrom
norbert/danger-fix
Nov 18, 2025
Merged

Danger: Fix the config file#33073
ndelangen merged 7 commits into
nextfrom
norbert/danger-fix

Conversation

@ndelangen
Copy link
Copy Markdown
Member

@ndelangen ndelangen commented Nov 18, 2025

What I did

  • Fix the Danger config file
  • Due to the previous change, node_modules are no longer installed, So I had to adapt a few things in order to make that work. I didn't want to re-introduce the install, because:
    • I kinda do not know how to, it's all in a single docker container, which we directly pass the path to the config-file to..
    • I like the fact that this makes the danger run faster
  • TS now checks the .js files
  • I had to disable the TS checks for previously written .js files individually
  • I added a fat comment with the config file restrictions that now ally.
  • I bumped the typescript package version and prettier, because I thought the problem I experienced were maybe due to those being outdated. (they were not, but I didn't feel like reverting that minor improvement)

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>

@ndelangen ndelangen changed the title Refactor Dangerfile: Consolidate label checks and improve readability Danger: Fix the config file Nov 18, 2025
@ndelangen ndelangen self-assigned this Nov 18, 2025
@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented Nov 18, 2025

View your CI Pipeline Execution ↗ for commit a9c37f6

Command Status Duration Result
nx run-many -t build --parallel=3 ✅ Succeeded 46s View ↗

☁️ Nx Cloud last updated this comment at 2025-11-18 11:43:43 UTC

…xtension and add new Dangerfile script for PR label checks
…xtension and add new Dangerfile script for PR label validation and title checks
@ndelangen ndelangen added ci:docs Run the CI jobs for documentation checks only. build Internal-facing build tooling & test updates labels Nov 18, 2025
Copy link
Copy Markdown
Member

@yannbf yannbf left a comment

Choose a reason for hiding this comment

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

LGTM. Can you make the PR fail on danger just to check its functionality? Like remove a label or something

…tension and add new Dangerfile script for PR label validation and title checks
@ndelangen ndelangen requested a review from yannbf November 18, 2025 11:21
Comment thread scripts/dangerfile.js
Comment on lines +1 to +14
/**
* IMPORTANT: This file has unique constraints due to how Danger.js executes it.
*
* Restrictions:
* - NO TypeScript: This file runs without any transpilation/transformation
* - NO external dependencies: Scripts dependencies are not installed in CI
* - NO Node.js built-ins: Even `fs` and other core modules don't work in Danger's runtime
* - MUST use `import` for Danger API: The Danger runtime only processes `import` statements,
* not `require()`. These imports get compiled to global references by Danger.js
* - CAN use `require()` for local files: Works for things like package.json
*
* Why: We want Danger to run as fast as possible in CI without installing dependencies
* or running build processes.
*/
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Just to highlight this for future reference, this comment is important.

There are restrictions on what you can do in this file.
Unfortunately those restriction are such that this is a FAUX ESM file!

@ndelangen ndelangen merged commit 3280e08 into next Nov 18, 2025
15 of 16 checks passed
@ndelangen ndelangen deleted the norbert/danger-fix branch November 18, 2025 11:31
@github-actions github-actions Bot mentioned this pull request Nov 18, 2025
17 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Internal-facing build tooling & test updates ci:docs Run the CI jobs for documentation checks only.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants