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

fix bug : cannot update param is struct type #106

Closed
wants to merge 1 commit into from

Conversation

ngoanhtan
Copy link
Contributor

Hi,
I Push an PR for fixing isssue

  • cannot update when the param input has a struct param

Here is example :

       var extendMetadata = &uiza.ExtendMetadata{
		MovieCategory: "1111111111",
		IMDBScore:     79500,
	}
	params := &uiza.EntityUpdateParams{
		ID:             uiza.String("entityID"),
		ExtendMetadata: extendMetadata,
	}
       response, err := entity.Update(params)
   It will parse extendMetadata to : extendMetadata[MovieCategory] :{IMDBScore:79500}. 

It will make the body of the request is wrong

{
   id:entityID,
   extendMetadata[IMDBScore]:79500,
   extendMetadata[MovieCategory]:1111111111,
}

It just correct it to :

{
   id:entityID,
   extendMetadata: {
      IMDBScore:79500,
      MovieCategory:1111111111
   }
}

@deathemperor
Copy link
Contributor

Please automize your commits. I saw you've also made changes to live.go but I don't get why.

Use fix bug or bug fix?

fix bug (verb + noun): indicates the action to fix a bug as in I fix a bug. Fix is a verb. The correct term should be "fix a bug"
bug fix (adj-noun + noun): indicate an action `I commited a bug fix. Fix is a noun.

In your format, you want to declare something "fix bug: cannot update param is struct type". Declaration should be a noun (fix bug is not), just like declaring a variable which should always be a noun.

Wording of the sentence can also be improved. The result can look like this:
bug fix: cannot update entity when param is a struct
or fixed a bug when param is a struct entity cannot be updated

@ngoanhtan
Copy link
Contributor Author

First, I am sorry for my English communication and very thank you by you correct me.
Second, I want clearly some issues below:

  • Please automize your commits. I saw you've also made changes to live.go but I don't get why.
    PushInfo and PullInfo structs are using for parsing from JSON response (with tag "json") and form request (with tag "form"). It missing "form" tag at here and I added it.
    I know you skip "form" tag and fixing by the other way as you implement live stream API but you make it different with other API (entity, category...) and I think you will do it faster than if you correct tag instead of implementing new UnmarshalJSON method.
  • About my pull request: when I work again with this project, I found this bug and want to fix it. Perfectly, if you fixed it in another way but I will keep my pull here just for some case someone needs it.

@deathemperor
Copy link
Contributor

I understand your concerns and thanks for the PR.

Just to be clear, I didn't make any changes regarding the PushInfo and PullInfo struct. The UnmarshalJSON was added NOT because of this either. It was to fixed a bug when linkStream = null as I've stated here f7e16ae.

I know you skip "form" tag and fixing by the other way as you implement live stream API but you make it different with other API (entity, category...) and I think you will do it faster than if you correct tag instead of implementing new UnmarshalJSON method.

Not sure what you meant by I skipped form tag. if you were talking about this commit, it was my mistake and I've corrected it (thanks for pointing it out, again).

Atomizing commits will help others to trace back issues when they happen. Could you please atomize this PR? I'm happy to merge after that.

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