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.

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

feat: add schema for test overrides #2

Merged
merged 4 commits into from
Feb 5, 2024
Merged

feat: add schema for test overrides #2

merged 4 commits into from
Feb 5, 2024

Conversation

theseion
Copy link
Collaborator

@theseion theseion commented Nov 9, 2023

No description provided.

@theseion
feat: add schema for test overrides
cd19db3
@theseion
feat: update overrides schema
8aae278 
- write and improve tests
- deprecate `log_contains` and `no_log_contains` in favour of new `log`
  struct
- add `expect_id` and `no_expect_id` to simplify standard log output
  matches
@theseion
feat: improve schema
2434b86 
- deprecate some fields in favor of new ones
- add `retry_once` option
@fzipi
Copy link
Member

fzipi commented Nov 20, 2023 •
edited
Loading

@theseion You are committing the generated files. I don't think we want that.

@fzipi fzipi self-requested a review November 20, 2023 23:13
@theseion
Copy link
Collaborator Author

Yeah, wasn't sure about that. I did it mainly because main.go requires the generated code. But the whole thing is a big hack anyway because dstdocgen can't deal with neither more than one schema nor reusing examples. So there's currently a manual step involved after generating the doc files.
I was going to talk to you about all this before completing this PR.

@theseion
Copy link
Collaborator Author

@fzipi I figured out a way to reuse code. I'd appreciate some feedback. The markdown output is still not perfect (references to external code are documented a bit weirdly), but in general it works well.

@fzipi
Copy link
Member

fzipi commented Jan 26, 2024

Awesome fix! Now I need to look at the weird stuff and compare. It took me some time the first iteration to get it in good shape...

@theseion
Copy link
Collaborator Author

theseion commented Feb 3, 2024

Can we merge this PR for now? I can't work on finishing the implementation in go-ftw to support the nginx tests.

fzipi
fzipi approved these changes Feb 5, 2024
View reviewed changes
Copy link
Member

@fzipi fzipi left a comment

Choose a reason for hiding this comment

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

Yes, let's move forward with this.

@fzipi fzipi merged commit c334dae into coreruleset:main Feb 5, 2024
1 check passed
@theseion theseion deleted the add-overrides-schema branch February 5, 2024 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fzipi fzipi fzipi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@theseion @fzipi