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

more ideomatic implementation, IMHO #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

oec
Copy link

@oec oec commented Oct 1, 2015

Hey Alessandro,

I got distracted by other tasks, coming back very late with this pull request we discussed earlier (sometime in July!). However, I'm not sure you are going to like it, as it changes quite a bit of the code:

  • rename package-name from 'goradius' -> 'radius'
  • initialization now with radius.New(server, port, pass)
  • AuthenticatorT is now named Authenticator
  • Authenticator struct exposes timeout and retries-attributes, so they can be changed
  • retry is actually implemented now
  • parsing the response correctly had been fixed in the meantime by some other contributor, but I have a different fix (which is shorter, I believe)
  • some things are more go-ideomatic, I think, but that is rather a matter of taste

Maybe you want to give it a try, or just stick with your (working!) version. As I said, for me it is more a matter of honoring your efforts by sharing my implementation.

Cheers,
Özgür

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

1 participant