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

v3 and root copies of the code desynced #520

Closed
maxb opened this issue May 27, 2024 · 5 comments
Closed

v3 and root copies of the code desynced #520

maxb opened this issue May 27, 2024 · 5 comments

Comments

@maxb
Copy link

maxb commented May 27, 2024

I think this is unintended:

--- ./conn.go   2024-05-27 20:22:00.225638254 +0000
+++ v3/conn.go  2024-05-27 20:22:00.229638338 +0000
@@ -535,9 +535,10 @@
                                l.messageContexts[message.MessageID] = message.Context

                                // Add timeout if defined
-                               if l.getTimeout() > 0 {
+                               requestTimeout := l.getTimeout()
+                               if requestTimeout > 0 {
                                        go func() {
-                                               timer := time.NewTimer(time.Duration(l.getTimeout()))
+                                               timer := time.NewTimer(time.Duration(requestTimeout))
                                                defer func() {
                                                        if err := recover(); err != nil {
                                                                l.err = fmt.Errorf("ldap: recovered panic in RequestTimeout: %v", err)
@johnweldon
Copy link
Member

What is your concern @maxb ? This seems like a reasonable change.

@johnweldon
Copy link
Member

Ah, you're saying there's a difference between the v3/conn.go and the ./conn.go

@johnweldon
Copy link
Member

I see @cpuschma caught this; we'll need to update the ./conn.go to match the v3/conn.go I suppose.

@maxb
Copy link
Author

maxb commented May 28, 2024

Quite... I wanted to open the can of worms of do we really need to continue having two copies of the code in the repo (in a separate issue), but making sure the two copies really are just duplicates is a necessary prerequisite.

cpuschma added a commit to cpuschma/ldap that referenced this issue May 28, 2024
As pointed out in go-ldap#520 (comment), the subdirectory v3 has been out of sync. This PR copies all missing changes to the root directory.
@cpuschma
Copy link
Member

We are already "discussing" our options regarding the two copies (see https://github.com/orgs/go-ldap/discussions/503). It is clear, and the incident has made it clear once again, that we have to stop doing this in the long term.

Thank you for pointing out the problem!

cpuschma added a commit that referenced this issue May 28, 2024
As pointed out in #520 (comment), the subdirectory v3 has been out of sync. This PR copies all missing changes to the root directory.
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

No branches or pull requests

3 participants