-
Notifications
You must be signed in to change notification settings - Fork 19
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
Development config #123
Development config #123
Conversation
Btw @0x4007 I don't have any permissions in the new orgs |
@@ -143,7 +144,7 @@ async function download({ context, repository, owner }: { context: GitHubContext | |||
const { data } = await context.octokit.rest.repos.getContent({ | |||
owner, | |||
repo: repository, | |||
path: CONFIG_FULL_PATH, | |||
path: context.eventHandler.environment === "production" ? CONFIG_FULL_PATH : DEV_CONFIG_FULL_PATH, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we should define the strings and import them for implementation?
export const environments = {
PRODUCTION: "production",
DEVELOPMENT: "development"
};
Although this can probably be done cleaner with some type of as const
or enum
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could add an enum to make it look better === Environment.PRODUCTION
but the type is already defined as "development" | "production"
so it's already type safe
I've been re-adding collaborators on an as-needed basis. Let me know which repo(s) you need elevated permissions for! |
It would be nice to have ability to request reviews, merge PRs, add labels |
@@ -12,6 +12,15 @@ import { shouldSkipPlugin } from "../src/github/handlers"; | |||
config({ path: ".dev.vars" }); | |||
|
|||
const issueOpened = "issues.opened"; | |||
const manifestPath = "manifest.json"; | |||
const repo = { | |||
owner: { login: "ubiquity" }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
owner: { login: "ubiquity" }, | |
owner: { login: "ubiquity-os-marketplace" }, |
Shouldn't it default to |
Resolves #122