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

Document calc(..., right) arg for parameter scales #266

Closed
wants to merge 2 commits into from

Conversation

MaxGhenis
Copy link
Contributor

No description provided.

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.

I understand that because of #267, many more changes than expected were introduced in this PR. In particular, I understand that only lines 144 and 145 implement the described change. I thus offer to replace this PR with another one until #267 is implemented, to speed up integration of this good contribution 🙂

Comment on lines +20 to +21
- `tax_on_salaries` , `tax_on_salary.public_sector` , `universal_income` are **nodes**;
- `tax_on_salaries.tax_scale` , `tax_on_salary.public_sector.rate` , `universal_income.minimum_age` , `universal_income.amount` are **parameters** (or scales).
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure about these added spaces before commas. I understand that even though those are code items, they should be read as part of a sentence. Could you please roll this back? 🙂

Comment on lines +303 to +305
>

```py
Copy link
Member

Choose a reason for hiding this comment

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

This seems inappropriate as it shifts from “quoted code” to “coded quote”.

@MattiSG
Copy link
Member

MattiSG commented Jun 16, 2022

@MaxGhenis I'll let you check that #268 indeed contains the entirety of the changes you intended, and close this PR if it is the case 🙂

@MaxGhenis MaxGhenis closed this Jun 16, 2022
@MaxGhenis MaxGhenis deleted the MaxGhenis/issue265 branch June 16, 2022 15:26
@MaxGhenis
Copy link
Contributor Author

MaxGhenis commented Jun 16, 2022

Thanks @MattiSG - yes, everything except those lines was introduced by my editor.

@MattiSG
Copy link
Member

MattiSG commented Jun 16, 2022

Thank you for your understanding and sorry for this being harder than it should!

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