Skip to content

August Updates#125

Merged
deepak2016 merged 2 commits intodevfrom
refresh-280818
Aug 29, 2018
Merged

August Updates#125
deepak2016 merged 2 commits intodevfrom
refresh-280818

Conversation

@deepak2016
Copy link
Copy Markdown
Contributor

@deepak2016 deepak2016 commented Aug 28, 2018 •
edited
Loading

@davidmoten
Copy link
Copy Markdown
Contributor

davidmoten commented Aug 28, 2018

@deepak2016 can you add the issue number to the "memory requirement issue fix" please

@davidmoten
Copy link
Copy Markdown
Contributor

davidmoten commented Aug 28, 2018

@deepak2016 Still seeing a bunch of star imports in the generated files. I would have thought this was an opportunity to get rid of them all. What's the story?

MIchaelMainer
MIchaelMainer approved these changes Aug 28, 2018
View reviewed changes
Copy link
Copy Markdown
Collaborator

@MIchaelMainer MIchaelMainer left a comment

Choose a reason for hiding this comment

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

Running the LongDescriptionAdder will remove the UTF-8 incompliant strings. Other than that, it is impossible to review all of the changes. Are there any linked generator changes for this PR?

@MIchaelMainer
Copy link
Copy Markdown
Collaborator

MIchaelMainer commented Aug 28, 2018

Good catch @davidmoten . It looks like the start imports work has been applied to the /requests and not the /models.

@deepak2016
Copy link
Copy Markdown
Contributor Author

deepak2016 commented Aug 29, 2018

Yes, have done changes so that SDK builds with less than 2GB RAM. Have not tried to fix everything.
Followed below approach to chose tempalates to fix:

  1. Create a list of template file vs number of java files getting generated against that template.
  2. Sort this list based on number of java files generated
  3. We ended up fixing ~25-30 templates out of ~80 templates to get maximum impact.

davidmoten
davidmoten approved these changes Aug 29, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@davidmoten davidmoten left a comment

Choose a reason for hiding this comment

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

I'll take your word for it, too big for full review and the real test is if CI passes.

@davidmoten
Copy link
Copy Markdown
Contributor

davidmoten commented Aug 29, 2018

I'd like to see a link to the generator changes too.

@deepak2016 deepak2016 mentioned this pull request Aug 29, 2018
Merged
@deepak2016 deepak2016 merged commit b7090fb into dev Aug 29, 2018
@davidmoten davidmoten mentioned this pull request Aug 30, 2018
Merged
@MIchaelMainer MIchaelMainer deleted the refresh-280818 branch July 3, 2020 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MIchaelMainer MIchaelMainer MIchaelMainer approved these changes

+2 more reviewers

@davidmoten davidmoten davidmoten approved these changes

@NakulSabharwal NakulSabharwal NakulSabharwal approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Memory requirements Import wildcards

4 participants

@deepak2016 @davidmoten @MIchaelMainer @NakulSabharwal