-
Notifications
You must be signed in to change notification settings - Fork 23
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
Fix documentation #251
Fix documentation #251
Conversation
baa03a2
to
69b6fe9
Compare
2222787
to
0ecac53
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good on my side.
@HAEKADI could you please confirm this fixes the documented problem? 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for the review @HAEKADI ! In fact, I discovered that half of the doc was still missing 😄 , I've corrected it and it should normally work as expected now. |
Yes it does :) |
@HAEKADI exactly, the tricky part of the situation is that as the documentation is in a different repository, we're forced to stage changes somehow to avoid the chicken-egg problems. What I propose in openfisca/openfisca-core#1038 is the easiest way I could imagine to achieve that, without involving complex platform-dependent workflows, obscure git features, and so on. Please do not hesitate if you think of another way to solve the specific core → doc problem to tell me —I did of course thought about re-merging together core and the doc, which is the standard for many libraries, but I intend to propose it separately as it is a major change just for a bug fix IMHO. |
@maukoquiroga I agree :) Looking at the changes, it might seem like it is redundant to have them in both repos |
2e3b808
to
0a46bff
Compare
Fixes #243
Fixes #244
Depends on openfisca/openfisca-core#1038
Bug Fix
Demo: