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

DM-49117: CI - Run all notebooks in PR #433

Merged
merged 1 commit into from
Feb 25, 2025

Conversation

fajpunk
Copy link
Member

@fajpunk fajpunk commented Feb 24, 2025

Not just the ones changed in the latest commit. Fixes this situation:

  • PR with notebooks a, and b gets submitted
  • mobu check fails with errors in notebooks a and b
  • A new commit that fixes a but NOT b is pushed
  • mobu check runs ONLY with a because it's the only notebook that was changed in the most recent commit
  • mobu check passes because a is fixed... but b is still broken
  • GitHub will say everything is good with the PR

Needs lsst-sqre/safir#387

@fajpunk fajpunk force-pushed the tickets/DM-49117/ci-all-notebooks-in-pr branch 3 times, most recently from 0820157 to c860596 Compare February 24, 2025 20:26
@fajpunk fajpunk requested a review from rra February 24, 2025 20:26
Copy link
Member

@rra rra left a comment

Choose a reason for hiding this comment

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

Nice, good fix.

@fajpunk fajpunk force-pushed the tickets/DM-49117/ci-all-notebooks-in-pr branch from c860596 to 104e7cc Compare February 25, 2025 04:26
@fajpunk fajpunk enabled auto-merge February 25, 2025 04:27
@fajpunk fajpunk merged commit 4897f45 into main Feb 25, 2025
4 checks passed
@fajpunk fajpunk deleted the tickets/DM-49117/ci-all-notebooks-in-pr branch February 25, 2025 04:28
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