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.

Sign up for GitHub

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

Jump to bottom

Continue self-hosted rename in build-related files #30483

Merged
merged 1 commit into from
Dec 8, 2021
Merged

Continue self-hosted rename in build-related files #30483

merged 1 commit into from
Dec 8, 2021

Conversation

chadwhitacre
Copy link
Member

@chadwhitacre chadwhitacre commented Dec 7, 2021 •
edited
Loading

Follow-up to #30482, part of getsentry/self-hosted#796.

Review

  • I need to look into the .craft.yml setting to make sure it's fine to change - thread
  • Pretty sure the cloudbuild.yml changes are fine.
  • What's up with the newsletter thing? Does anyone actually consume that? Bit of a sidecar here but didn't seem worth a separate PR and not really related better to anything else.—Dead code. Removing separately.

Todo

chadwhitacre
chadwhitacre commented Dec 7, 2021
View reviewed changes
.craft.yml
@@ -6,7 +6,7 @@ statusProvider:
name: github
config:
contexts:
- 'onpremise-builder (sentryio)'
- 'self-hosted-builder (sentryio)'
Copy link
Member Author

Choose a reason for hiding this comment

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

https://github.com/getsentry/craft#status-provider

Copy link
Member Author

Choose a reason for hiding this comment

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

        const contextResult = this.getStatusForContext(
          contextString,
          revisionStatus,
          revisionCheckSuites,
          revisionChecks
        );

https://github.com/getsentry/craft/blob/1e352b9e6fd486abc16f7f249b243929b26dd679/src/status_providers/github.ts#L69-L74

Copy link
Member Author

Choose a reason for hiding this comment

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

E.g. https://github.com/getsentry/sentry/runs/4448300537

Where is it on this PR? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

Is this where I have to do /gcbrun? 🧐

Copy link
Member Author

Choose a reason for hiding this comment

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

Alright yeah this is where I start to fall off in terms of permissions/visibility ... I gather that there is some manually configured job name in GCR ... 🤨

Copy link
Member Author

Choose a reason for hiding this comment

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

where I start to fall off in terms of permissions/visibility

OR DO I!?!?!?!?!?!

Screen Shot 2021-12-07 at 5 25 27 PM

💃

Copy link
Member Author

Choose a reason for hiding this comment

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

Of course, Google may very well show me a "Save" button but throw an error if I click it. 🙄

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, I can edit. 😁

Screen Shot 2021-12-07 at 5 29 50 PM

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay! Sooooooooo ... when this PR lands I intend to edit the two triggers in GCB. I'll keep an eye on this during the 21.12.0 release ... kicked out getsentry/self-hosted#1193 for that.

@chadwhitacre chadwhitacre mentioned this pull request Dec 7, 2021
Closed
42 tasks
@chadwhitacre
Copy link
Member Author

Why am I not seeing a GCB build on this PR? 🤔

@chadwhitacre
Copy link
Member Author

I see a build for #30482. Maybe it just takes a while?

@chadwhitacre
Copy link
Member Author

Hrm, I definitely want to see onpremise-github-pr-check succeed here, since that consumes docker/cloudbuild.yml, which changes on this PR.

Base automatically changed from cwlw/self-hosted-rename-safe to master December 7, 2021 22:51
@visual-snapshot visual-snapshot bot requested a review from a team as a code owner December 7, 2021 22:51
@chadwhitacre chadwhitacre force-pushed the cwlw/self-hosted-rename-buildy-stuff branch from 897d5da to aae6f2e Compare December 7, 2021 22:54
@chadwhitacre
Copy link
Member Author

Rebased and I'm getting a build now. ¯\_(ツ)_/¯

@chadwhitacre chadwhitacre force-pushed the cwlw/self-hosted-rename-buildy-stuff branch from aae6f2e to 2830797 Compare December 7, 2021 23:10
@chadwhitacre chadwhitacre mentioned this pull request Dec 7, 2021
Merged
@chadwhitacre chadwhitacre force-pushed the cwlw/self-hosted-rename-buildy-stuff branch from 2830797 to e8f6787 Compare December 8, 2021 17:05
@chadwhitacre
Copy link
Member Author

Rebased to pick up #30501. 🤞

@chadwhitacre chadwhitacre enabled auto-merge (squash) December 8, 2021 17:12
@chadwhitacre
Copy link
Member Author

Is there anything else that depends on the name onpremise-builder? 🤔

@chadwhitacre
Copy link
Member Author

No, if GitHub code search is to be trusted.

@chadwhitacre chadwhitacre merged commit 83b15f3 into master Dec 8, 2021
@chadwhitacre chadwhitacre deleted the cwlw/self-hosted-rename-buildy-stuff branch December 8, 2021 18:29
@chadwhitacre
Copy link
Member Author

Triggers renamed. 👍

@chadwhitacre chadwhitacre mentioned this pull request Dec 8, 2021
Closed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@joshuarli joshuarli joshuarli approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@chadwhitacre @joshuarli