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

Merge DummyCountry and Country Template #4

Merged
merged 43 commits into from
May 17, 2017
Merged

Conversation

fpagnoux
Copy link
Member

@fpagnoux fpagnoux force-pushed the merge-dummy-boilerplate branch from ef4adcf to 8cb3297 Compare May 16, 2017 19:55
@fpagnoux fpagnoux requested a review from MattiSG May 16, 2017 19:55
@fpagnoux fpagnoux force-pushed the merge-dummy-boilerplate branch 6 times, most recently from 0a4ffbd to 2fc4778 Compare May 16, 2017 22:02
Copy link
Member

@MattiSG MattiSG left a comment

Choose a reason for hiding this comment

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

Wow. This moved me. I'm super proud of this work.
I believe this will be a stepping stone to the “contributability” of OpenFisca, as well as a new landmark in the quality standard of our production.
Please accept my sincere congratulations.

.gitignore Outdated
.project
.project
.pydevproject
.pydevproject
Copy link
Member

Choose a reason for hiding this comment

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

Several duplicated lines.

.gitignore Outdated
.spyderproject
.vscode
build
dist
Copy link
Member

Choose a reason for hiding this comment

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

It is a good practice to group and name blocks, in order to increase readability and maintainability of the list.

Examples:

CONTRIBUTING.md Outdated
@@ -0,0 +1,98 @@
> This files defines the rules to follow to contribute to your repository.
Copy link
Member

Choose a reason for hiding this comment

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

“This file”.

README.md Outdated
@@ -19,32 +18,50 @@ git clone https://github.com/openfisca/country-template.git # download this tem
mv country-template openfisca-$lowercase_country_name
cd openfisca-$lowercase_country_name
git remote remove origin
sed -i '' "s/country_template/$lowercase_country_name/g" MANIFEST.in openfisca_country_template/base.py openfisca_country_template/model.py
sed -i '' "s/Country-Template/$COUNTRY_NAME/g" setup.py
sed -i '' '3,27d' README.md
Copy link
Member

Choose a reason for hiding this comment

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

Add comments to explain what this does.

README.md Outdated

> We recommend that you [use a virtualenv](https://doc.openfisca.fr/for_developers.html#create-a-virtualenv) to install OpenFisca. If you don't, you may need to add `--user` at the end of all commands starting by `pip`.

To install your country package, run:

```
pip install -e .
pip install -e ".[test]"
```

You can make sure that everything is working by running the provided test:
Copy link
Member

Choose a reason for hiding this comment

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

tests

<!-- See https://doc.openfisca.fr/parameters.html -->
<NODE code="general">
<CODE code="age_of_majority" description="Age of majority">
<!-- This parameter has changed on the 1st of Jan 1971. -->
Copy link
Member

Choose a reason for hiding this comment

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

“has changed” → “has last changed”

<!-- Unlike variables, parameters are not specific to a certain person or household. They are set by the legislation on depend on time. -->
<!-- See https://doc.openfisca.fr/parameters.html -->
<NODE code="general">
<CODE code="age_of_majority" description="Age of majority">
Copy link
Member

Choose a reason for hiding this comment

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

“Age of majority” → “Age of majority (in years)”

#! /usr/bin/env python
# -*- coding: utf-8 -*-

# This file browses the country package to find yaml tests and runs them.
Copy link
Member

Choose a reason for hiding this comment

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

YAML

label = "Basic income provided to adults"
url = "https://law.gov.example/basic_income" # Always use the most official source

# Since Dec 1st 2016, the basic income is provided to any adult, without cousidering their income.
Copy link
Member

Choose a reason for hiding this comment

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

considering

column = IntCol
entity = Person
definition_period = MONTH
label = u"Person's age"
Copy link
Member

Choose a reason for hiding this comment

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

(in years)

@MattiSG MattiSG self-assigned this May 17, 2017
@MattiSG MattiSG removed their assignment May 17, 2017
@fpagnoux fpagnoux force-pushed the merge-dummy-boilerplate branch from d4c133c to 1024f76 Compare May 17, 2017 17:34
@fpagnoux fpagnoux merged commit b2f9f8a into master May 17, 2017
@fpagnoux fpagnoux deleted the merge-dummy-boilerplate branch May 17, 2017 17:37
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.

2 participants