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

5524 add pick list reports #5535

Merged
merged 14 commits into from
Dec 19, 2024
Merged

5524 add pick list reports #5535

merged 14 commits into from
Dec 19, 2024

Conversation

fergie-nz
Copy link
Contributor

@fergie-nz fergie-nz commented Nov 22, 2024

Fixes #5534

πŸ‘©πŸ»β€πŸ’» What does this PR do?

Adds remaining standard (picklist) reports

πŸ’Œ Any notes for the reviewer?

I'm unsure of the context / subcontext requirements of the pick list reports.
Awaiting discussion to confirm

πŸ§ͺ Testing

πŸ“ƒ Documentation

  • Part of an epic: documentation will be completed for the feature as a whole

@fergie-nz fergie-nz changed the base branch from develop to v2.4.0 November 22, 2024 22:22
Copy link

github-actions bot commented Nov 22, 2024

Bundle size difference

Comparing this PR to main

Old size New size Diff
4.99 MB 4.99 MB 170 B (0.00%)

@github-actions github-actions bot added this to the V2.4.0-RC4 milestone Nov 24, 2024
@github-actions github-actions bot added enhancement New feature or request Priority: Must Have The product will not work without this feature: reports labels Nov 24, 2024
@fergie-nz fergie-nz linked an issue Nov 26, 2024 that may be closed by this pull request
@roxy-dao roxy-dao changed the base branch from v2.4.0 to develop November 28, 2024 22:46
@roxy-dao roxy-dao modified the milestones: V2.4.0-RC6, v2.5.0 Nov 28, 2024
@github-actions github-actions bot modified the milestones: v2.5.0, V2.4.0-RC6 Nov 28, 2024
@roxy-dao roxy-dao modified the milestones: V2.4.0-RC6, V2.4.0-RC7, v2.5.0 Nov 28, 2024
Copy link
Collaborator

@andreievg andreievg left a comment

Choose a reason for hiding this comment

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

Can you please remove convert_data_js, we would only add them when used/needed.

I've tried to run outbound report, had problems, need to add the following to see what the issue is: 5524-add-pick-list-reports...5524-add-pick-list-reports(Better-logging-for-report-generation)
I know it's not something that you would have created, but good to see this error (can you add to this please)

Also can you please add an -o flag or something like that to UpsertReportsJson, to upsert reports in dev mode (overwrite existing reports).

I am keen to get this approved, we can decide on the types we want in reports before 2.5 is out

"install",
"--cwd",
&convert_dir,
"--no-lockfile",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ohh wow on second run it's much faster, personally I would optimise this by running in parallel, but I don't think we should do this especially since I would pivot from extism to BoaJS

@fergie-nz fergie-nz marked this pull request as ready for review December 19, 2024 02:31
Copy link
Collaborator

@andreievg andreievg left a comment

Choose a reason for hiding this comment

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

Thanks looks great

@fergie-nz fergie-nz merged commit 92432f9 into develop Dec 19, 2024
5 checks passed
@fergie-nz fergie-nz deleted the 5524-add-pick-list-reports branch December 19, 2024 03:56
@fergie-nz
Copy link
Contributor Author

Can you please remove convert_data_js, we would only add them when used/needed.

I've tried to run outbound report, had problems, need to add the following to see what the issue is: 5524-add-pick-list-reports...5524-add-pick-list-reports(Better-logging-for-report-generation) I know it's not something that you would have created, but good to see this error (can you add to this please)

Also can you please add an -o flag or something like that to UpsertReportsJson, to upsert reports in dev mode (overwrite existing reports).

I am keen to get this approved, we can decide on the types we want in reports before 2.5 is out

Followup issues for CLI added here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature: reports Priority: Must Have The product will not work without this
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add remaining standard reports to OMS repo (pick-list reports)
3 participants