Skip to content

Updates for the upcoming new Insights outputs#4

Merged
oschwald merged 20 commits into
masterfrom
dave/new-minfraud-outputs
Jan 21, 2016
Merged

Updates for the upcoming new Insights outputs#4
oschwald merged 20 commits into
masterfrom
dave/new-minfraud-outputs

Conversation

@autarch

@autarch autarch commented Jan 18, 2016

Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need these empty constructors for anything (also in the Email class)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was mostly cutting and pasting here. Can I just move the @JsonProperty to the attribute declaration instead?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd leave it on the constructor parameter.

Comment thread CHANGELOG.md Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These are private. You should probably rephrase this in terms of the getter. Same with the warning attributes.

oschwald added a commit that referenced this pull request Jan 21, 2016
Updates for the upcoming new Insights outputs
@oschwald
oschwald merged commit 63912b2 into master Jan 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants