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

294 define codelist in qubeconfig #460

Merged
merged 81 commits into from
May 27, 2022

Conversation

GDonRanasinghe
Copy link
Contributor

@GDonRanasinghe GDonRanasinghe commented Apr 26, 2022

This PR involves extending the qube config so that the users can define code lists within the qube config file.

The following tasks are ready for review:

  1. New json representation of a codelist (see schema/codelist-config/v1_0/codelistconfig-example.jsonc file).
  2. New schema.json for the creating and validating a codelist (see schema/codelist-config/v1_0/schema.json file).
  3. Alter the code which parses the JSON so that it extracts the relevant information and puts it into the code list class.
  4. Implementations for handling versioning for cube config so that v1.0 and v1.1 of cube config works.
  5. Ability to define inline code lists.
  6. Unit and behave tests.

Satisfies #294

@GDonRanasinghe GDonRanasinghe self-assigned this Apr 26, 2022
@GDonRanasinghe GDonRanasinghe marked this pull request as ready for review April 27, 2022 14:32
code list
Copy link
Contributor

@canwaf canwaf left a comment

Choose a reason for hiding this comment

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

There's a bit more work which needs doing to this to get it ready for coding; think of how we would follow a predictable "configuration by convention" approach as we have done with cube configs.

@GDonRanasinghe GDonRanasinghe removed the request for review from robons May 3, 2022 16:19
@GDonRanasinghe GDonRanasinghe marked this pull request as draft May 3, 2022 16:20
Copy link
Contributor

@robons robons left a comment

Choose a reason for hiding this comment

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

Looks pretty close to being done.

If you could change the schema a little or use enums in the sort object as well as changing the underlying behaviour of the sort functionality then that would make it great.

@GDonRanasinghe
Copy link
Contributor Author

Looks pretty close to being done.

If you could change the schema a little or use enums in the sort object as well as changing the underlying behaviour of the sort functionality then that would make it great.

@robons Sure, I will do these updates.

Copy link
Contributor

@robons robons left a comment

Choose a reason for hiding this comment

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

Looks pretty good. So long as you add some tests for the sort_order logic, you're good to go.

@GDonRanasinghe
Copy link
Contributor Author

Looks pretty close to being done.

If you could change the schema a little or use enums in the sort object as well as changing the underlying behaviour of the sort functionality then that would make it great.

@robons This is also done now.

@GDonRanasinghe GDonRanasinghe merged commit 1a43499 into main May 27, 2022
@GDonRanasinghe GDonRanasinghe deleted the 294-define-codelist-in-qubeconfig branch May 27, 2022 11:45
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