drift-org / backend

2 stars 0 forks source link

Expanded mgm user model #13

Closed aidenszeto closed 3 years ago

aidenszeto commented 3 years ago

Contributors

@aidenszeto

Reviewers

@brandonLi8 @teekenzie

Relevant issue

Closes #8

Summary of change

Testing/Verification

teekenzie commented 3 years ago

Why do we need the 0 in the array. I don't think we need those right?

aidenszeto commented 3 years ago

Why do we need the 0 in the array. I don't think we need those right?

I think the 0 defaults it to an empty array? I'm pretty sure array sizes have to be known by compiler at compile time too

teekenzie commented 3 years ago

I think the 0 defaults it to an empty array? I'm pretty sure array sizes have to be known by compiler at compile time too

On second thought, you actually have to remove those. You are basically telling Go that we can only have a 0 length array placed into there.

By not having any number, you could create a dynamic slice and it would still be treated as an array in mongo database

aidenszeto commented 3 years ago

I think the 0 defaults it to an empty array? I'm pretty sure array sizes have to be known by compiler at compile time too

On second thought, you actually have to remove those. You are basically telling Go that we can only have a 0 length array placed into there.

By not having any number, you could create a dynamic slice and it would still be treated as an array in mongo database

Ok yeah I forgot that you can have dynamic slices in Go, changes should be pushed

teekenzie commented 3 years ago

Also, since you have slice involved in your struct, please also include the creating function. For how to write this function, please refer to the demo-app

aidenszeto commented 3 years ago

Also, since you have slice involved in your struct, please also include the creating function. For how to write this function, please refer to the demo-app

That's something we can do in a different ticket. This issue is for expanding the user model

teekenzie commented 3 years ago

That's something we can do in a different ticket. This issue is for expanding the user model

I think this is something that should be included in this ticket. Since this is simply making sure that we won't be running into errors when inserting into database.

aidenszeto commented 3 years ago

That's something we can do in a different ticket. This issue is for expanding the user model

I think this is something that should be included in this ticket. Since this is simply making sure that we won't be running into errors when inserting into database.

I think we should just merge this in and include the route in a different PR if we wanna stick to one ticket = one PR

aidenszeto commented 3 years ago

I think we should just merge this in and include the route in a different PR if we wanna stick to one ticket = one PR

I actually agree with @teekenzie on this one. Adding the creating function is associated with your changes and this ticket.

Ok yeah I misinterpreted what was meant by the creating function. Changes are pushed.