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

[Fixes #125] Dump examples as json #130

Merged
merged 1 commit into from
Apr 30, 2014

Conversation

johanneswuerbach
Copy link
Contributor

As I had several encoding issues while loading the recorded examples from the yaml file and converting them to json in the documentation, I changed the default storage format to json.

I added also some tests for the writer to ensure the result is now that same as before. You can use the apipie:convert_examples task to convert the existing examples.

@johanneswuerbach
Copy link
Contributor Author

Any update on this? The PR fixed all unparsable example files for us.

@iNecas
Copy link
Member

iNecas commented Sep 5, 2013

Hi, sry for the delay, I need to update our projects to work with this change and changed the version to 0.x.0 because it's more than just a bug-fix, therefore this delay. Would a release sometime the following two weeks work for you?

@johanneswuerbach
Copy link
Contributor Author

Sure I was just asking, whether I should change something :) Thanks for the update 👍

@johanneswuerbach
Copy link
Contributor Author

@iNecas any update on this?

@iNecas
Copy link
Member

iNecas commented Dec 20, 2013

Man, I feel so bad about this one. And in general, I'm horrible person on maintaining this gem. Please be patient with me. I have a major release of apipie with this change included on my new-years resolution. Also waiting for couple of additions for API/CLI bindings based on the docs.

@johanneswuerbach
Copy link
Contributor Author

@iNecas as 0.1.0 is now released 😎, should I rebase or close the PR?

@mtparet
Copy link
Contributor

mtparet commented Mar 3, 2014

As MultiJson is used, you should add multi_json as a dependency of the gem, that's why tests are failing under Rails 3.0 and 4.1.
Anyway 👍

@johanneswuerbach
Copy link
Contributor Author

Rails and other gems removed the multi_json dependency. Should I also do that? What are the supported rubies of this gem? 1.9.3+?

@mtparet
Copy link
Contributor

mtparet commented Mar 3, 2014

Yes, so we should perhaps use the json gem instead of multi_json.

@johanneswuerbach
Copy link
Contributor Author

Updated and passing :-)

@iNecas
Copy link
Member

iNecas commented Mar 4, 2014

Thanks, I would like to get this and #187 in and do 0.2.0 later this week. Does that work for you?

@johanneswuerbach
Copy link
Contributor Author

Sure, #187 looks awesome :-)

@@ -17,6 +17,7 @@ Gem::Specification.new do |s|
s.require_paths = ["lib"]

s.add_dependency "rails", ">= 3.0.10"
s.add_dependency "json", ">= 1.7"
Copy link
Member

Choose a reason for hiding this comment

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

Any particular reason for making this version requirement?

Copy link
Member

Choose a reason for hiding this comment

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

It worked for me with JSON 1.5.5 as well, I would remove that if you don't mind

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rails 4.1 is requiring 1.7.7 due to a fixed security issue. rails/rails@78cd3b0

Copy link
Member

Choose a reason for hiding this comment

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

The problem is, it's not unusual that software vendors (e.g. Red Hat provides security patches to older version of software, so it looks like the version is 1.5.5, but the security issues are being backported. Therefore, ideally, the version constraints should be there only in cases it requires the functionality that was not before.

@johanneswuerbach
Copy link
Contributor Author

ok, rebased and version requirement removed.

@iNecas
Copy link
Member

iNecas commented Apr 30, 2014

Nice, the only thing pending is the travis failing on ruby 2.1, which is not related to this PR, once travis fixes that, I will merge that finally! Thanks for patience with me :)

@johanneswuerbach
Copy link
Contributor Author

Sure, just send you #235 , this fixes travis.

iNecas added a commit that referenced this pull request Apr 30, 2014
@iNecas iNecas merged commit 4b65fea into Apipie:master Apr 30, 2014
@iNecas
Copy link
Member

iNecas commented Apr 30, 2014

That should be the solution! Thanks for both

@johanneswuerbach
Copy link
Contributor Author

Yeah 🎉 😄

@iNecas
Copy link
Member

iNecas commented May 12, 2014

And as promised, 0.2.0 is out. Almost on time :D Thanks for the patience again

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