Skip to content

Beautyfied json #21

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
Closed
wants to merge 4 commits into from
Closed

Beautyfied json #21

wants to merge 4 commits into from

Conversation

@glimow
Copy link
Contributor

@glimow glimow commented May 8, 2019

glimow added 3 commits April 24, 2019 15:04
@glimow
Add id_splitter_nn/522bdd11-d1fa-49dd-9e51-87c529283418
45069c7 
Signed-off-by: tristan kalos <tristan.kalos@live.fr>
@glimow
add reference to original paper
8a79062 
Signed-off-by: tristan kalos <tristan.kalos@live.fr>
@glimow
update model description
0801e1c 
Signed-off-by: tristan kalos <tristan.kalos@live.fr>
@glimow
Copy link
Contributor Author

glimow commented May 8, 2019

@vmarkovtsev

vmarkovtsev
vmarkovtsev requested changes May 8, 2019
View reviewed changes
Copy link
Collaborator

@vmarkovtsev vmarkovtsev left a comment

Choose a reason for hiding this comment

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

https://github.com/src-d/modelforge/blob/master/doc/contributing.md#format-of-the-commit-message

vmarkovtsev
vmarkovtsev requested changes May 8, 2019
View reviewed changes
Copy link
Collaborator

@vmarkovtsev vmarkovtsev left a comment

Choose a reason for hiding this comment

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

Not the same as json.dump(indent=4, sort_keys=True)

@glimow
Copy link
Contributor Author

glimow commented May 10, 2019

src-d/modelforge#96 @vmarkovtsev removed the sort_keys that we introduced in last PR.
I also ensured the JSON in this PR matches the one produced by modelforge

@vmarkovtsev
Copy link
Collaborator

vmarkovtsev commented May 18, 2019

All the json-s everywhere must be indent=4, sort_keys=True @glimow

@vmarkovtsev
Copy link
Collaborator

vmarkovtsev commented May 21, 2019

I think that sort_keys=True is still missing.

@glimow
Sort and indent index.json
eb63977 
Signed-off-by: tristan kalos <tristan.kalos@live.fr>
vmarkovtsev
vmarkovtsev requested changes May 22, 2019
View reviewed changes
Copy link
Collaborator

@vmarkovtsev vmarkovtsev left a comment

Choose a reason for hiding this comment

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

There should be 2 separate PRs:

  • Readable JSON
  • Adding the new model

The current one contains both.

@glimow glimow closed this May 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vmarkovtsev vmarkovtsev vmarkovtsev requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@glimow @vmarkovtsev