Skip to content

feat(models): add report filename option - #174

Merged
BioPhoton merged 15 commits into
mainfrom
add-report-filename-option
Oct 30, 2023
Merged

BioPhoton merged 15 commits into
mainfrom
add-report-filename-option

Conversation

@BioPhoton

@BioPhoton BioPhoton commented Oct 28, 2023 •

Copy link
Copy Markdown
Collaborator

The changes should help to improve testing of report generation. ATM all tests write in the same folder and with the same file name. Adding a filename option to core would improve the testing situation and is helpful in for integrations.

This PR includes:

Implementing the CLI option is resolved in a followup PR

closes #161

@BioPhoton BioPhoton mentioned this pull request Oct 28, 2023
2 tasks
@BioPhoton BioPhoton added ➕ enhancement new feature or request 🔬 testing writing tests 🧩 core labels Oct 29, 2023
@BioPhoton
BioPhoton requested a review from matejchalk October 29, 2023 12:06
Comment thread packages/models/src/lib/implementation/utils.spec.ts Outdated
Comment thread packages/models/src/lib/implementation/utils.spec.ts Outdated
Comment thread packages/models/src/lib/implementation/schemas.ts Outdated
Comment thread e2e/cli-e2e/tests/print-config.spec.ts Outdated
Comment thread e2e/cli-e2e/tests/print-config.spec.ts Outdated
Comment thread packages/models/test/fixtures/report.mock.ts
BioPhoton and others added 5 commits October 29, 2023 18:31
Co-authored-by: Matěj Chalk <34691111+matejchalk@users.noreply.github.com>
Co-authored-by: Matěj Chalk <34691111+matejchalk@users.noreply.github.com>
Co-authored-by: Matěj Chalk <34691111+matejchalk@users.noreply.github.com>
Co-authored-by: Matěj Chalk <34691111+matejchalk@users.noreply.github.com>
Co-authored-by: Matěj Chalk <34691111+matejchalk@users.noreply.github.com>
@BioPhoton BioPhoton self-assigned this Oct 29, 2023
@BioPhoton
BioPhoton requested a review from matejchalk October 29, 2023 18:48

@matejchalk matejchalk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It works nicely with the config 👍

The PR description is a bit misleading, --filename (or --persist.filename) doesn't work as a CLI argument. You'd have to add persist.filename to core-config-options.ts to make it accessible from the CLI (we should auto-sync those in the future, probably).

Also, I noticed the PR title doesn't follow our commit conventions - model scope should be models.

matejchalk
matejchalk previously approved these changes Oct 30, 2023
vmasek
vmasek previously approved these changes Oct 30, 2023
@BioPhoton
BioPhoton dismissed stale reviews from vmasek and matejchalk via db2c06d October 30, 2023 11:43
@BioPhoton BioPhoton changed the title feat(model): add report filename option refactor(model): add report filename option Oct 30, 2023
@BioPhoton
BioPhoton requested a review from matejchalk October 30, 2023 13:52
@BioPhoton BioPhoton changed the title refactor(model): add report filename option feat(models): add report filename option Oct 30, 2023
@BioPhoton
BioPhoton merged commit bdeab54 into main Oct 30, 2023
@BioPhoton
BioPhoton deleted the add-report-filename-option branch October 30, 2023 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🧩 core ➕ enhancement new feature or request 🔬 testing writing tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add optional filename to PersistConfig

3 participants