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.

Sign up for GitHub

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

Jump to bottom

Task/part2 rendering of entities #1288

Merged
merged 4 commits into from
Sep 25, 2015
Merged

Task/part2 rendering of entities #1288

merged 4 commits into from
Sep 25, 2015

Conversation

kzangeli
Copy link
Member

Part II of issue #1259, rendering the output of /v2/entities[/< eid >]

Three renering modes implemented:

  • normalized (default),
  • keyValues, and
  • values (tentative - format is not even fully decided yet - I think)

The mechanism right now to pick a rendering mode is via URL parameters.
This is not really decided yet - might change to URL PATH instead.
But, that is a minor change in the implementation.

@kzangeli kzangeli added this to the 0.25.0 milestone Sep 24, 2015
fgalan
fgalan reviewed Sep 25, 2015
View reviewed changes
src/lib/apiTypesV2/Entity.cpp

if (ciP->uriParamOptions["keyValues"] == true)
{
renderMode = "keyValues";
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunatelly, "keyValues" seems that will not be used at the end. However, the issue is still under discussion, so probably the best strategy is to progress this PR without changes, then fix it in a next PR once it gets clear. What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I really believe that would be easier.

Copy link
Member

Choose a reason for hiding this comment

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

At the end seems that "keyValues" will be used. Keep tunned :)

NTC

@fgalan
Copy link
Member

fgalan commented Sep 25, 2015

LGTM (although I have to admit that the "identation changes noisy" make me not be too confident about my review...)

fgalan pushed a commit that referenced this pull request Sep 25, 2015
Merge pull request #1288 from telefonicaid/task/part2_rendering_of_en…
d9d0f5b 
…tities

Task/part2 rendering of entities
@fgalan fgalan merged commit d9d0f5b into feature/1259_new_json_for_v2 Sep 25, 2015
@fgalan fgalan deleted the task/part2_rendering_of_entities branch September 25, 2015 14:07
@kzangeli
Copy link
Member Author

Sorry about that.
It's just ... so extremely difficult to fix a failing functest (or source code for that matter) that is so badly indented that you have problems reading it.
I understand your point and I agree. At the same time ... It's difficult. I promise to try ...

Any case, if you ever have doubts on functests, execute them in your own machine.
It's not 100% proof but it means a lot that they actually work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@fgalan fgalan

Labels
None yet
Projects
None yet
Milestone
0.25.0
Development

Successfully merging this pull request may close these issues.
2 participants
@kzangeli @fgalan