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

✨ Quartz Energy v0.1.2 → Staging #553

Open
wants to merge 21 commits into
base: staging
Choose a base branch
from
Open

✨ Quartz Energy v0.1.2 → Staging #553

wants to merge 21 commits into from

Conversation

braddf
Copy link
Contributor

@braddf braddf commented Jan 15, 2025

Pull Request

Description

Much trial and error, but Cypress tests should hopefully now be working in Github Actions.
This PR includes:

  • Quartz Solar:
    • Fixes to the Node CI action/setup
    • Cypress Cloud now set up to capture CI runs and debug any failures 🙌
    • Upgrade Sentry on Quartz Solar to match new Quartz Energy version
  • Quartz Energy:
    • Add Sentry to Quartz Energy
    • Add Sentry Message when user downloads CSV on Quartz Energy

Fixes #552

How Has This Been Tested?

Locally with nektos/act, and development branch on Github

  • Yes

Checklist:

  • My code follows OCF's coding style guidelines
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked my code and corrected any misspellings

@braddf braddf added bug Something isn't working enhancement New feature or request labels Jan 15, 2025
@braddf braddf requested a review from peterdudfield January 15, 2025 16:35
@braddf braddf self-assigned this Jan 15, 2025
Copy link

vercel bot commented Jan 15, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
quartz-app ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 21, 2025 11:18am
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
nowcasting-app ⬜️ Ignored (Inspect) Visit Preview Jan 21, 2025 11:18am

@braddf braddf changed the title Fix Node CI ✨ Quartz Energy v0.1.2 – Staging Jan 21, 2025
@braddf braddf changed the title ✨ Quartz Energy v0.1.2 – Staging →✨ Quartz Energy v0.1.2 arrow Staging Jan 21, 2025
@braddf braddf changed the title →✨ Quartz Energy v0.1.2 arrow Staging ✨ Quartz Energy v0.1.2 → Staging Jan 21, 2025
@@ -1,49 +1,77 @@
name: Node CI

on: [push]
on: [pull_request]
Copy link
Contributor

Choose a reason for hiding this comment

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

sure we just want to run this on PRs? Does it take a long time to run, is that why you want to reduce it?

fi
# Now only runs unit tests, Cypress tests are run in the CI pipeline on PRs
yarn test

Copy link
Contributor

Choose a reason for hiding this comment

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

do unit tests get run in CI too?

maskAllText: true,
blockAllMedia: true
})
].filter((integration) => integration.name !== "Dedupe");
Copy link
Contributor

Choose a reason for hiding this comment

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

dedupe?

import { EventHint, ErrorEvent } from "@sentry/nextjs";

Sentry.init({
dsn: "https://[email protected]/4507622592151552",
Copy link
Contributor

Choose a reason for hiding this comment

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

should these be hard coded? Or env vars?

import { EventHint, ErrorEvent } from "@sentry/nextjs";

Sentry.init({
dsn: "https://[email protected]/4507622592151552",
Copy link
Contributor

Choose a reason for hiding this comment

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

similar point to earlier, should we make this an en var?

Copy link
Contributor

@peterdudfield peterdudfield left a comment

Choose a reason for hiding this comment

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

Looks good. I left a few comments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants