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.

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

Update slices.SortFunc for Go 1.21 #4706

Merged
merged 3 commits into from Aug 25, 2023
Merged

Update slices.SortFunc for Go 1.21 #4706

merged 3 commits into from Aug 25, 2023

Conversation

evanj
Copy link
Contributor

@evanj evanj commented Jul 31, 2023

What changed?

The golang.org/x/exp module was updated to use the same function signature as Go 1.21's upcoming slices.SortFunc function. This changed the return value from a bool (less than) to an int (compare to). Update Temporal to the latest version and fix the uses.

Why?

For more details about the upstream change, see the following issue and commit:

How did you test it?

I ran the tests locally on my machine.

Potential risks

Worst case I may have switched the sort order somewhere accidentally.

Is hotfix candidate?

No

The golang.org/x/exp module was updated to use the same function
signature as Go 1.21's upcoming slices.SortFunc function. This changed
the return value from a bool (less than) to an int (compare to). Update
Temporal to the latest version and fix the uses.

For more details about this change, see the following issue and commit:

- golang/go#61374
- https://cs.opensource.google/go/x/exp/+/302865e7556b4ae5de27248ce625d443ef4ad3ed
@evanj evanj requested a review from a team as a code owner July 31, 2023 18:14
@CLAassistant
Copy link

CLAassistant commented Jul 31, 2023

CLA assistant check
All committers have signed the CLA.

@evanj
Copy link
Contributor Author

evanj commented Jul 31, 2023

Hm I thought Datadog has a CLA already. Let me ask around internally about that.

@evanj
Copy link
Contributor Author

evanj commented Jul 31, 2023

Hmmm Datadog definitely has a Temporal CLA signed already. I think the problem may be that I need to open the pull request from the DataDog github account; let me see if I can work out the permissions I may need.

Copy link
Member

@yycptt yycptt left a comment

Choose a reason for hiding this comment

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

cc @yiminc re. the CLA

service/history/queues/action_pending_task_count.go Outdated Show resolved Hide resolved
This simplifies the code from a code review suggestion.

Co-authored-by: Yichao Yang <yycptt@gmail.com>
@evanj
Copy link
Contributor Author

evanj commented Aug 9, 2023

Re CLA: Do you want to me to open this pull request again, from the Datadog github account? I believe I can do that. This work was done as a Datadog employee, so the CLA should be signed.

I just updated the branch in the hopes that the test failures were flakes.

@yiminc
Copy link
Member

yiminc commented Aug 11, 2023

@evanj we do need to get the CLA signed in order to get this pr merged.

@yiminc yiminc merged commit 4a4667b into temporalio:main Aug 25, 2023
10 checks passed
xwduan pushed a commit that referenced this pull request Aug 29, 2023
<!-- Describe what has changed in this PR -->
**What changed?**

The golang.org/x/exp module was updated to use the same function
signature as Go 1.21's upcoming slices.SortFunc function. This changed
the return value from a bool (less than) to an int (compare to). Update
Temporal to the latest version and fix the uses.


<!-- Tell your future self why have you made these changes -->
**Why?**

For more details about the upstream change, see the following issue and
commit:

- golang/go#61374
-
https://cs.opensource.google/go/x/exp/+/302865e7556b4ae5de27248ce625d443ef4ad3ed


<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**

I ran the tests locally on my machine.

<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**

Worst case I may have switched the sort order somewhere accidentally.


<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**

No

---------

Co-authored-by: Yichao Yang <yycptt@gmail.com>
wxing1292 pushed a commit to wxing1292/temporal that referenced this pull request Sep 27, 2023
<!-- Describe what has changed in this PR -->
**What changed?**

The golang.org/x/exp module was updated to use the same function
signature as Go 1.21's upcoming slices.SortFunc function. This changed
the return value from a bool (less than) to an int (compare to). Update
Temporal to the latest version and fix the uses.


<!-- Tell your future self why have you made these changes -->
**Why?**

For more details about the upstream change, see the following issue and
commit:

- golang/go#61374
-
https://cs.opensource.google/go/x/exp/+/302865e7556b4ae5de27248ce625d443ef4ad3ed


<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**

I ran the tests locally on my machine.

<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**

Worst case I may have switched the sort order somewhere accidentally.


<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**

No

---------

Co-authored-by: Yichao Yang <yycptt@gmail.com>
bergundy pushed a commit to bergundy/temporal that referenced this pull request Oct 17, 2023
<!-- Describe what has changed in this PR -->
**What changed?**

The golang.org/x/exp module was updated to use the same function
signature as Go 1.21's upcoming slices.SortFunc function. This changed
the return value from a bool (less than) to an int (compare to). Update
Temporal to the latest version and fix the uses.


<!-- Tell your future self why have you made these changes -->
**Why?**

For more details about the upstream change, see the following issue and
commit:

- golang/go#61374
-
https://cs.opensource.google/go/x/exp/+/302865e7556b4ae5de27248ce625d443ef4ad3ed


<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**

I ran the tests locally on my machine.

<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**

Worst case I may have switched the sort order somewhere accidentally.


<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**

No

---------

Co-authored-by: Yichao Yang <yycptt@gmail.com>
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.

None yet

4 participants