Skip to content

feat(entities): add role builder #923

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
Draft
wants to merge 1 commit into
base: refactor/cleanup-entities
Choose a base branch
from
Draft

feat(entities): add role builder #923

wants to merge 1 commit into from

Conversation

bonjourmauko
Copy link
Member

@bonjourmauko bonjourmauko commented Jan 2, 2020 •
edited
Loading

Depends on #1286

@bonjourmauko bonjourmauko added kind:fix Bugs are defects and failure demand. level:intermediate Requires average OpenFisca experience labels Jan 2, 2020
@bonjourmauko bonjourmauko requested review from a team and benjello January 2, 2020 04:12
@bonjourmauko bonjourmauko added level:advanced Requires substantial OpenFisca experience and removed level:intermediate Requires average OpenFisca experience labels Jan 2, 2020
@bonjourmauko bonjourmauko mentioned this pull request Jan 2, 2020
Merged
5 tasks
Morendil
Morendil reviewed Jan 2, 2020
View reviewed changes
Copy link
Contributor

@Morendil Morendil left a comment

Choose a reason for hiding this comment

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

I would avoid large-scale style changes in a PR ostensibly focused on a targeted bug fix. I lack the energy and time available to hunt for the substantial changes.

benjello
benjello reviewed Jan 2, 2020
View reviewed changes
Copy link
Member

@benjello benjello left a comment

Choose a reason for hiding this comment

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

I might not be the best qualified person but I read the PR and didn't find anything bizarre.

bonjourmauko
bonjourmauko commented Jan 2, 2020
View reviewed changes
@bonjourmauko
Copy link
Member Author

This PR adds tests, code itself doesn't change, except for :

len(x)

to

size(x)

Question is : handling scalar vectors is the expected behaviour ?

@bonjourmauko bonjourmauko mentioned this pull request Jan 2, 2020
Closed
@bonjourmauko bonjourmauko added kind:refactor Refactoring and code cleanup level:starter Suited for beginners and new contributors and removed kind:fix Bugs are defects and failure demand. level:advanced Requires substantial OpenFisca experience labels Jan 2, 2020
@bonjourmauko bonjourmauko changed the title Fix enum scalar array bug Add tests to Enum and to EnumArray Jan 2, 2020
@bonjourmauko
Copy link
Member Author

Hey thanks @Morendil and @benjello!

Now PR is scoped to document expected behaviour of Enum and EnumArray.

bonjourmauko
bonjourmauko commented Jan 2, 2020
View reviewed changes
bonjourmauko
bonjourmauko commented Jan 2, 2020
View reviewed changes
bonjourmauko
bonjourmauko commented Jan 3, 2020
View reviewed changes
@bonjourmauko bonjourmauko force-pushed the fix-enum-scalar-array-bug branch from 61e6358 to 465d5bf Compare January 3, 2020 14:38
@bonjourmauko
Copy link
Member Author

Bump @Morendil @benjello

@sandcha sandcha self-requested a review February 13, 2020 11:44
@bonjourmauko bonjourmauko force-pushed the fix-enum-scalar-array-bug branch from 465d5bf to 1c14773 Compare October 12, 2020 20:31
@bonjourmauko bonjourmauko changed the base branch from master to doctests/entities September 7, 2021 20:29
@bonjourmauko bonjourmauko changed the title Add tests to Enum and to EnumArray Improve enums module doctests Sep 7, 2021
@bonjourmauko bonjourmauko force-pushed the fix-enum-scalar-array-bug branch from 1c14773 to 7a7de05 Compare September 7, 2021 21:20
@bonjourmauko bonjourmauko removed kind:theme A group of issues, directly tied to an OKR kind:roadmap A group of issues, constituting a delivery roadmap labels Oct 7, 2021
@bonjourmauko bonjourmauko force-pushed the doctests/entities branch 4 times, most recently from 17d128c to 845235d Compare October 28, 2021 11:05
@bonjourmauko bonjourmauko force-pushed the fix-enum-scalar-array-bug branch from 47d447c to 175efd4 Compare October 4, 2024 21:00
@bonjourmauko bonjourmauko changed the title [5/17] Improve enums module doctests test: improve enums module doctests Oct 4, 2024
@bonjourmauko bonjourmauko force-pushed the fix-enum-scalar-array-bug branch 2 times, most recently from 7db7564 to ee318d5 Compare October 5, 2024 01:46
@bonjourmauko bonjourmauko changed the base branch from doctests/entities to master October 5, 2024 02:55
@bonjourmauko bonjourmauko force-pushed the fix-enum-scalar-array-bug branch from ee318d5 to 3a147bd Compare October 5, 2024 19:40
@bonjourmauko bonjourmauko changed the base branch from master to docs/add-docs-to-enums October 5, 2024 19:40
@bonjourmauko bonjourmauko changed the base branch from docs/add-docs-to-enums to test/fix-enums-doctests October 9, 2024 01:24
@bonjourmauko bonjourmauko force-pushed the test/fix-enums-doctests branch 2 times, most recently from 05e9db7 to 6dfc93f Compare October 9, 2024 02:37
@bonjourmauko bonjourmauko force-pushed the fix-enum-scalar-array-bug branch from 3a147bd to 7e1513e Compare October 10, 2024 22:15
@bonjourmauko bonjourmauko changed the base branch from test/fix-enums-doctests to fix-mypy-checks-enums October 10, 2024 22:16
@bonjourmauko bonjourmauko changed the title test: improve enums module doctests feat(entities): add role builder Oct 10, 2024
@bonjourmauko bonjourmauko force-pushed the fix-mypy-checks-enums branch 7 times, most recently from 0ca8e75 to 9b7d9c9 Compare October 14, 2024 11:33
Base automatically changed from fix-mypy-checks-enums to master October 14, 2024 11:38
@bonjourmauko bonjourmauko changed the base branch from master to refactor/cleanup-entities October 25, 2024 19:02
@bonjourmauko bonjourmauko force-pushed the refactor/cleanup-entities branch from 1011e8b to 1459e31 Compare November 19, 2024 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@benjello benjello benjello left review comments

@Morendil Morendil Awaiting requested review from Morendil

@sandcha sandcha Awaiting requested review from sandcha

@MattiSG MattiSG Awaiting requested review from MattiSG

@HAEKADI HAEKADI Awaiting requested review from HAEKADI

Assignees

No one assigned

Labels

kind:refactor Refactoring and code cleanup

Projects

None yet

Milestone

Improved testing

Development

Successfully merging this pull request may close these issues.

3 participants

@bonjourmauko @Morendil @benjello