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

review feat(model): annotes getter and setter with a CtRole #1377

Merged
merged 11 commits into from
Jun 9, 2017

Conversation

tdurieux
Copy link
Collaborator

@tdurieux tdurieux commented Jun 7, 2017

This PR is related to #1368

TODO tests

@tdurieux tdurieux force-pushed the model_getter_setter branch from 4a24766 to 37ffe1c Compare June 8, 2017 09:12
@tdurieux tdurieux changed the title WIP feat(model): annotes getter and setter with a CtRole review feat(model): annotes getter and setter with a CtRole Jun 8, 2017
@tdurieux tdurieux force-pushed the model_getter_setter branch from 37ffe1c to 39b0a9d Compare June 8, 2017 09:20
@monperrus
Copy link
Collaborator

per our discussion

  • type is for expression type (no comment, no type access)
  • no role in reference interfaces
  • remove contract all getters annotated
  • split MEMBER in CONSTRUCTOR/FIELD/EXECUTABLE/NESTED_TYPE

@INRIA INRIA deleted a comment from spoon-bot Jun 8, 2017
@INRIA INRIA deleted a comment from spoon-bot Jun 8, 2017
@INRIA INRIA deleted a comment from spoon-bot Jun 8, 2017
@INRIA INRIA deleted a comment from spoon-bot Jun 8, 2017
@monperrus monperrus force-pushed the model_getter_setter branch from fb1af05 to 9919c91 Compare June 8, 2017 20:28
@monperrus
Copy link
Collaborator

@surli OK for merge for me, please check.

This will probably be adjusted later, but this is a really good start, thanks @tdurieux

DEFAULT_EXPRESSION,
THEN,
ELSE,
PACKAGE,
SUB_PACKAGE,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not keeping PACKAGE here, it seems clearer?

@monperrus
Copy link
Collaborator

because it is more explicit, the role is to be a subpackage of another one

@surli
Copy link
Collaborator

surli commented Jun 9, 2017

because it is more explicit, the role is to be a subpackage of another one

Ok I saw the usage after asking.

@surli surli merged commit 841edfc into INRIA:master Jun 9, 2017
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.

3 participants