Skip to content

Add pr checks#5

Merged
DinneK merged 21 commits intomainfrom
add-pr-checks
Mar 6, 2026
Merged

Add pr checks#5
DinneK merged 21 commits intomainfrom
add-pr-checks

Conversation

@DinneK
Copy link
Collaborator

@DinneK DinneK commented Jan 7, 2026

Problem

A path issue existed in the issues/new urls and extra "/" was being created.

Solution

  • Change href="https://{{ site.baseurl }}/{{ org }}/metrics/issues/new" in banner.liquid to href="{{ site.githubBaseUrl}}/{{ org }}/metrics/issues/new" this ensure the url behaves the same in both private and public CMS repos.
  • Add githubBaseUrl to site.js

Result

Path issue fixed.

Next steps

  • Merge into Metrics as module
  • Resolve linting and scans in the next PR
  • Once merged into metrics add env variable to metrics_internal

DinneK and others added 19 commits January 7, 2026 09:59
Signed-off-by: Dinne Kopelevich <dinne.kopelevich@gsa.gov>
Signed-off-by: Dinne Kopelevich <dinne.kopelevich@gsa.gov>
Signed-off-by: Dinne Kopelevich <dinne.kopelevich@gsa.gov>
Signed-off-by: Dinne Kopelevich <dinne.kopelevich@gsa.gov>
Signed-off-by: Dinne Kopelevich <dinne.kopelevich@gsa.gov>
Signed-off-by: Dinne Kopelevich <dinne.kopelevich@gsa.gov>
Signed-off-by: Dinne Kopelevich <dinne.kopelevich@gsa.gov>
Signed-off-by: Dinne Kopelevich <dinne.kopelevich@gsa.gov>
Signed-off-by: Dinne Kopelevich <dinne.kopelevich@gsa.gov>
Signed-off-by: Dinne Kopelevich <dinne.kopelevich@gsa.gov>
Signed-off-by: Dinne Kopelevich <dinne.kopelevich@gsa.gov>
Signed-off-by: Dinne Kopelevich <dinne.kopelevich@gsa.gov>
Signed-off-by: Dinne Kopelevich <dinne.kopelevich@gsa.gov>
Signed-off-by: Dinne Kopelevich <dinne.kopelevich@gsa.gov>
Signed-off-by: Dinne Kopelevich <dinne.kopelevich@gsa.gov>
Signed-off-by: Dinne Kopelevich <dinne.kopelevich@gsa.gov>
Signed-off-by: Dinne Kopelevich <dinne.kopelevich@gsa.gov>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link

@natalialuzuriaga natalialuzuriaga left a comment

Choose a reason for hiding this comment

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

Just a question double checking a variable but otherwise LGTM!

const baseurl = isProduction ? "/metrics" : ""

// For modification between public and private repos
const githubBaseUrl = isProduction ? "https://github.cms.gov" : "https://github.com"

Choose a reason for hiding this comment

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

Just wanted to check, is the isProduction variable the right flag to determine the githubBaseUrl?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ooh, this is a really good catch. This solves for production, but not for development. I think the cleanest solution is adding an environmental variable and doing the same in the private metrics repo. I'll add here and then to metrics_internal.

Copy link

@decause-gov decause-gov left a comment

Choose a reason for hiding this comment

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

Once Nat's question is addressed, LGTM +1 🚢

DinneK added 2 commits March 3, 2026 10:45
Signed-off-by: Dinne Kopelevich <dinne.kopelevich@gsa.gov>
Signed-off-by: Dinne Kopelevich <dinne.kopelevich@gsa.gov>
Comment on lines +11 to +12
executablePath: "/usr/bin/google-chrome"
}
Copy link

Choose a reason for hiding this comment

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

[prettier] reported by reviewdog 🐶

Suggested change
executablePath: "/usr/bin/google-chrome"
}
executablePath: "/usr/bin/google-chrome",
},

Comment on lines +14 to +17
urls: [
"http://localhost:8080"
]
} No newline at end of file
Copy link

Choose a reason for hiding this comment

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

[prettier] reported by reviewdog 🐶

Suggested change
urls: [
"http://localhost:8080"
]
}
urls: ["http://localhost:8080"],
}

@DinneK DinneK requested a review from natalialuzuriaga March 3, 2026 17:52
Copy link

@natalialuzuriaga natalialuzuriaga left a comment

Choose a reason for hiding this comment

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

LGTM!

@DinneK DinneK merged commit dfc1698 into main Mar 6, 2026
10 of 14 checks passed
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.

3 participants