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

Added implementation of x-json-ignore #390

Merged
merged 1 commit into from Oct 7, 2022

Conversation

sparshev
Copy link
Contributor

The implementation of x-go-json-ignore to use with GORM when we don't need to serialize specific fields (when ObjectID and Object fields is in the same struct).

@sparshev
Copy link
Contributor Author

Hi @deepmap-marcinr, could you please suggest who can check the PR? If it's good - I can work on the tests and other required improvements to make sure it will fit perfectly.

@deepmap-marcinr
Copy link
Contributor

I'm traveling on vacation right now, I'll take a look soon.

@sparshev
Copy link
Contributor Author

Hi @deepmap-marcinr could you please check this one?

@@ -465,6 +472,7 @@ func GenFieldsFromProperties(props []Property) []string {
tags[i] = fmt.Sprintf(`%s:"%s"`, k, fieldTags[k])
}
field += "`" + strings.Join(tags, " ") + "`"

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -51,3 +52,18 @@ func extExtraTags(extPropValue interface{}) (map[string]string, error) {
}
return tags, nil
}

func extParseGoJsonIgnore(extPropValue interface{}) (bool, error) {

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Collaborator

@jamietanna jamietanna left a comment

Choose a reason for hiding this comment

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

Looking good! Would you mind updating the branch, to make sure CI is happy?

We will look at confirming functionality wise it looks good / whether we need any new tests

@sparshev
Copy link
Contributor Author

I actually thought this change will never be here... That probably will be a pain, but let me try to recall what happened here.

@sparshev
Copy link
Contributor Author

Ok, I removed the logic optimization and left just the code related to the json-ignore. @jamietanna could you please approve the workflows?

@sparshev
Copy link
Contributor Author

I hope it's not another year to wait...

@sparshev
Copy link
Contributor Author

sparshev commented Oct 7, 2022

@jamietanna may I ask you to check this again? The workflows seems doesn't want to start until someone will approve them.

@deepmap-marcinr deepmap-marcinr merged commit 53ffd8e into deepmap:master Oct 7, 2022
@sparshev sparshev deleted the json_ignore_extension branch October 7, 2022 03:41
adrianpk pushed a commit to foorester/oapi-codegen that referenced this pull request Jan 16, 2024
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

3 participants