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

Add user group support #234

Merged
merged 1 commit into from
Sep 13, 2016
Merged

Add user group support #234

merged 1 commit into from
Sep 13, 2016

Conversation

arichardet
Copy link
Contributor

Adds support for user group:

  • Add a user group given the name and gid
  • Delete a user group given the name and gid

Includes sample/group.hcl
If no "state" is included, the default is "present"

"github.com/asteris-llc/converge/resource"
)

const (
Copy link
Contributor

@BrianHicks BrianHicks Sep 12, 2016

Choose a reason for hiding this comment

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

it's typical for an enum like this to define a custom string alias and then use that, to make it clear that non any 'ol string will do.

type State string

const Present State = "present"

// and then...

type Group struct {
    State State
}

Copy link
Contributor

Choose a reason for hiding this comment

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

oh and they should probably be prefixed StatePresent etc


const (
GID_MIN = 1000
GID_MAX = 32767
Copy link
Contributor

Choose a reason for hiding this comment

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

could you document these constants, BTW? Just a comment will do. They're kind of random-looking otherwise

if err != nil {
return nil, err
}
if gidVal >= math.MaxUint32 {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you need this validation. Setting the bit size parameter in the ParseUint call above should take care of it. https://play.golang.org/p/E1b_Asq0WQ

@arichardet
Copy link
Contributor Author

PR feedback addressed.

@BrianHicks BrianHicks merged commit 69167c0 into master Sep 13, 2016
@BrianHicks BrianHicks deleted the feature/user-group branch September 13, 2016 20:27
BrianHicks added a commit that referenced this pull request Dec 22, 2016
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.

2 participants