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

Add warnings and deprecate ackWarningsList and warningsList #92

Merged
merged 1 commit into from
Mar 14, 2018

Conversation

jsom
Copy link
Contributor

@jsom jsom commented Mar 13, 2018

No description provided.

@jsom jsom force-pushed the jrs/#155954025/add-new-warnings branch from 6f073ac to c06416d Compare March 14, 2018 13:17
import it.trade.model.reponse.Warning;

public class TradeItOrderDetailsParcelable implements Parcelable {
public String orderSymbol;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@guillaumedebavelaere I updated to move warnings to the orderDetails. What's our strategy for accessors? Half the classes seem to define them for other fields but then others don't and just mark the field as public.

Copy link
Contributor

Choose a reason for hiding this comment

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

If the user is supposed to modify these fields you can let them public. Otherwise, you can mark them private (or package-private/protected if you need to set them in unit tests) and add a getter.

@jsom jsom force-pushed the jrs/#155954025/add-new-warnings branch from c06416d to 5a7309b Compare March 14, 2018 13:40
@jsom
Copy link
Contributor Author

jsom commented Mar 14, 2018

@guillaumedebavelaere updated with protected fields and public getters.

import it.trade.model.reponse.WarningLink;

public class TradeItWarningLinkParcelable implements Parcelable {
protected String label;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add the getters?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

import it.trade.model.reponse.WarningLink;

public class TradeItWarningParcelable implements Parcelable {
protected String message;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add the getters?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@jsom jsom force-pushed the jrs/#155954025/add-new-warnings branch from 5a7309b to b5a1d1d Compare March 14, 2018 14:09
@jsom jsom merged commit a9210a3 into develop Mar 14, 2018
@jsom jsom deleted the jrs/#155954025/add-new-warnings branch March 14, 2018 15:54
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