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

batches: use user name/e-mail with unauthored batch specs#46385

Merged
adeola-ak merged 4 commits intomainfrom
aa/add-author
Jan 19, 2023
Merged

batches: use user name/e-mail with unauthored batch specs#46385
adeola-ak merged 4 commits intomainfrom
aa/add-author

Conversation

@adeola-ak
Copy link
Contributor

@adeola-ak adeola-ak commented Jan 12, 2023

fixes the E2E test that failed after merge in #43828 and needed to be reverted

Implementing the same fallback behaviour for changeset specs on SSBC that we have on the client side where we try to get the author name and e-mail for commits from the environment if they're not in the batch spec by pulling that from the user's account details and created a sub-package author to do so

Test plan

Tested created a bc in various cases with missing information

@cla-bot cla-bot bot added the cla-signed label Jan 12, 2023
@sourcegraph-bot
Copy link
Contributor

sourcegraph-bot commented Jan 12, 2023

Codenotify: Notifying subscribers in CODENOTIFY files for diff 4807df1...706182c.

Notify File(s)
@bobheadxi enterprise/dev/ci/integration/executors/tester/main.go
@efritz enterprise/cmd/worker/internal/batches/workers/batch_spec_workspace_creator.go
@eseliger enterprise/cmd/worker/internal/batches/workers/batch_spec_workspace_creator.go
enterprise/internal/batches/store/author/author.go
enterprise/internal/batches/store/author/author_test.go
enterprise/internal/batches/store/worker_workspace_execution.go

@sourcegraph-bot
Copy link
Contributor

sourcegraph-bot commented Jan 12, 2023

Codenotify: Notifying subscribers in OWNERS files for diff 4807df1...706182c.

Notify File(s)
@sourcegraph/dev-experience enterprise/dev/ci/integration/executors/tester/main.go

@adeola-ak adeola-ak requested a review from a team January 12, 2023 19:31
Copy link
Contributor

@courier-new courier-new 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 to me, but could we trigger the executor e2e tests for this PR to double-check before we merge?

@adeola-ak
Copy link
Contributor Author

@courier-new how can i trigger the executor e2e tests?

@github-actions
Copy link
Contributor

Problem: the label i-acknowledge-this-goes-into-the-release is absent.
👉 What to do: we're in the next Sourcegraph release code freeze period. If you are 100% sure your changes should get released or provide no risk to the release, add the label your PR with i-acknowledge-this-goes-into-the-release.

@courier-new
Copy link
Contributor

From your branch locally you can run sg ci build main-dry-run. You should see the full buildkite suite with executors e2e kick off from that. 🙂

@adeola-ak
Copy link
Contributor Author

nice, thanks! looks like its good beside security scans

Copy link
Contributor

@courier-new courier-new left a comment

Choose a reason for hiding this comment

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

Awesome thanks! I'll approve to unblock, but I also think there's no rush on this if you want to wait until after 4.4 cut tomorrow.

@adeola-ak
Copy link
Contributor Author

thank uuu! yes thats what i was thinking, ill just wait

@adeola-ak adeola-ak merged commit ad3cd86 into main Jan 19, 2023
@adeola-ak adeola-ak deleted the aa/add-author branch January 19, 2023 18:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants