Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Backend: remove internalClient.ExternalURL#55463

Merged
camdencheek merged 4 commits into
mainfrom
cc/remove-exturl
Aug 2, 2023
Merged

Backend: remove internalClient.ExternalURL#55463
camdencheek merged 4 commits into
mainfrom
cc/remove-exturl

Conversation

@camdencheek

Copy link
Copy Markdown
Member

This removes the internalClient.ExternalURL method, which can be replaced with a simple call to conf.Get()

Test plan

Tested that the links in a code monitor still point to the right external URL

@cla-bot cla-bot Bot added the cla-signed label Jul 31, 2023
@camdencheek camdencheek marked this pull request as ready for review July 31, 2023 23:05
@sourcegraph-bot

sourcegraph-bot commented Jul 31, 2023

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff f9d67fe...cf8f8e7.

Notify File(s)
@eseliger internal/batches/reconciler/BUILD.bazel
internal/batches/reconciler/executor_test.go
internal/batches/reconciler/reconciler_test.go
internal/batches/types/BUILD.bazel
internal/batches/types/batch_change.go
internal/batches/types/batch_change_test.go

@camdencheek camdencheek force-pushed the cc/remove-exturl branch 2 times, most recently from 536c8a7 to 1497948 Compare July 31, 2023 23:13

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

there's cmd/frontend/globals/globals.go::ExternalURL that kinda does exactly what's in here, plus reacts to config changes. Should we move that to out of cmd/frontend and use it in here as well?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, we should. I think this can all just live in the conf package. It's really just caching the URL parsing.

Base automatically changed from cc/remove-internal-sendemail to main August 2, 2023 18:30
@camdencheek camdencheek enabled auto-merge (squash) August 2, 2023 18:40
@camdencheek camdencheek merged commit c7ab586 into main Aug 2, 2023
@camdencheek camdencheek deleted the cc/remove-exturl branch August 2, 2023 18:52
@github-actions

github-actions Bot commented Aug 2, 2023

Copy link
Copy Markdown
Contributor

The backport to 5.1 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-5.1 5.1
# Navigate to the new working tree
cd .worktrees/backport-5.1
# Create a new branch
git switch --create backport-55463-to-5.1
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 c7ab5860fe7597795bc237b4e1574e948c4ebfd6
# Push it to GitHub
git push --set-upstream origin backport-55463-to-5.1
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-5.1

Then, create a pull request where the base branch is 5.1 and the compare/head branch is backport-55463-to-5.1.

@github-actions github-actions Bot added backports failed-backport-to-5.1 release-blocker Prevents us from releasing: https://about.sourcegraph.com/handbook/engineering/releases labels Aug 2, 2023
varsanojidan pushed a commit that referenced this pull request Aug 3, 2023
This removes the `internalClient.ExternalURL` method, which can be
replaced with a simple call to `conf.Get()`
davejrt pushed a commit that referenced this pull request Aug 9, 2023
This removes the `internalClient.ExternalURL` method, which can be
replaced with a simple call to `conf.Get()`
camdencheek added a commit that referenced this pull request Aug 17, 2023
This removes the `internalClient.ExternalURL` method, which can be
replaced with a simple call to `conf.Get()`
camdencheek added a commit that referenced this pull request Aug 17, 2023
* Backend: remove internalClient.SendEmail (#55459)

Now that config can be fetched from all services, there is no need for
this internalClient method. One fewer to implement for gRPC

(cherry picked from commit f9d67fe)

* gRPC: migrate conf endpoint (#55648)

(cherry picked from commit 1dd8601)

* Backend: remove internalClient.ExternalURL (#55463)

This removes the `internalClient.ExternalURL` method, which can be
replaced with a simple call to `conf.Get()`
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

backports cla-signed release-blocker Prevents us from releasing: https://about.sourcegraph.com/handbook/engineering/releases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants