kaizen: prepare for v1.0.0 #167

timbray · 2022-12-31T18:30:39Z

fixes: #153

fixes: #144

fixes: #121

fixes: #81
Signed-off-by: Tim Bray tbray@textuality.com

Did more or less all the stuff listed in #153, mostly trivial cleanup, removed a lot of TODOs, pulled in pieces of two event-ruler unit tests (one found a bug!).

I will try in future to avoid this sort of grab-bag PR.

codecov-commenter · 2022-12-31T18:55:58Z

Codecov Report

Base: 94.48% // Head: 95.93% // Increases project coverage by +1.44% 🎉

Coverage data is based on head (0b9ff15) compared to base (014af06).
Patch coverage: 97.43% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #167      +/-   ##
==========================================
+ Coverage   94.48%   95.93%   +1.44%     
==========================================
  Files          17       17              
  Lines        2068     2067       -1     
==========================================
+ Hits         1954     1983      +29     
+ Misses         80       60      -20     
+ Partials       34       24      -10

Impacted Files	Coverage Δ
anything_but.go	`95.23% <ø> (ø)`
field_matcher.go	`100.00% <ø> (ø)`
list_maker.go	`100.00% <ø> (ø)`
live_pattern_state.go	`100.00% <ø> (ø)`
segments_tree.go	`100.00% <ø> (ø)`
shell_style.go	`86.00% <ø> (ø)`
pattern.go	`96.17% <90.90%> (+17.03%)`	⬆️
core_matcher.go	`95.94% <100.00%> (+0.14%)`	⬆️
flatten_json.go	`95.44% <100.00%> (+0.34%)`	⬆️
pruner.go	`97.98% <100.00%> (ø)`
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

timbray · 2022-12-31T19:30:07Z

Hmm, the unit tests were initially failing because of non-case-sensitive filesystem on MacOS (testData vs testdata) so I fixed that.

Now they're failing with error code 143, I think we're hitting actions/runner-images#6680

Blecch. Remove a couple of heavy tests I guess?

This makes sense because I added another pretty heavy benchmark from was-event-ruler.

timbray · 2022-12-31T21:59:06Z

Hmm, could segregate the borrowed-from-ruler tests and run them as a separate CI task, or step in a task? Looking for go test arguments to exclude/include files.

yosiat · 2023-01-01T11:12:21Z

Hmm, could segregate the borrowed-from-ruler tests and run them as a separate CI task, or step in a task? Looking for go test arguments to exclude/include files.

Looks like go don't have a way to ignore a specific test, there are two options here (from what I see):

Splitting tests by tags (https://mickey.dev/posts/go-build-tags-testing/) - for unit / integration / etc..
Split the tests to multiple packages - unit / integration and have CI step to run each of the packages.

Regarding the ported benchmark tests, maybe we should convert them to go benchmarks? and then they will run as part of "Benchmarks" CI step?

citylots_bench_test.go

flatten_json.go

generic_machine_test.go

yosiat · 2023-01-01T11:19:56Z

.chglog/config.yml

@@ -18,6 +18,7 @@ options:
      pat: 📖 Pattern Language
      chore: 🧹 Chore
      fix: 🐞 Fix
+      kaizen: 👩‍🎨 Improve


Maybe we should introduce commitlint as CI action to make sure we are following this guideline.

@embano1 what do you think?

Great idea!

cl2_test.go

timbray · 2023-01-01T18:20:09Z

Regarding the ported benchmark tests, maybe we should convert them to go benchmarks? and then they will run as part of "Benchmarks" CI step?

That's probably worth trying. Would it be possible to retain the existing printout, which I have arranged to compatible with aws/event-ruler, so that we can compare performance.

embano1 · 2023-01-03T08:15:31Z

.chglog/config.yml

@@ -18,6 +18,7 @@ options:
      pat: 📖 Pattern Language
      chore: 🧹 Chore
      fix: 🐞 Fix
+      kaizen: 👩‍🎨 Improve


Great idea!

embano1 · 2023-01-03T08:15:47Z

example_test.go

@@ -1,54 +0,0 @@
-package quamina_test


Why is was this file deleted?

There were no tests in it and nothing called it, dead code. Probably a leftover forgotten by one of us.

Ah, ok. Let me clarify: any file example_*_test.go in Go with functions prefixed with Example* inside serves as documentation examples rendered by Go doc, etc. See the current version for Quamina here: https://pkg.go.dev/github.com/timbray/quamina#example-New

You can even run this example in Go doc!

Besides serving as documentation, these are also tests including assertions (see // Output: pattern matched for event: "premium user" inside the file). If you run our tests, those will also run and fail if the example is not consistent with the code anymore - that's a great benefit of the Go test and documentation tooling :).

Please revert the deletion.

Oh, OK, I learned something today.

PATTERNS.md

README.md

cl2_test.go

PATTERNS.md

README.md

cl2_test.go

README.md

timbray · 2023-01-10T00:22:59Z

Folks, would it be OK to push this now and then address the CI problems with unit test and benchmark? Or should we do that cleanup first?

embano1 · 2023-01-10T07:57:04Z

Ok with me. Are you going to squash commits?

timbray · 2023-01-10T17:43:23Z

Ok with me. Are you going to squash commits?

Do you mean squash this #167 together with the CI-fix code? I don't think so, this PR lines up nicely with issue #153 (and ended up addressing a few more). "Preparing for 1.0" doesn't necessarily mean "final step before 1.0." Also it's a big PR with lots of code improvements and I'd like to land it.

embano1 · 2023-01-10T20:47:35Z

Do you mean squash this #167 together with the CI-fix code? I don't think so, this PR lines up nicely with issue #153 (and ended up addressing a few more). "Preparing for 1.0" doesn't necessarily mean "final step before 1.0." Also it's a big PR with lots of code improvements and I'd like to land it.

Nope, just asking whether you want to squash these 6 commits into one before merging :)

timbray · 2023-01-10T20:50:25Z

D'oh, I thought commit --amend did that. One of these years I must really buckle down and understand git better.

What is your opinion? And if we decide to do this, is there a good HOWTO somewhere?

embano1 · 2023-01-10T21:01:44Z

https://www.mgasch.com/2021/05/git-basics/#please-squash-your-commits :D

timbray · 2023-01-13T22:28:40Z

@embano1 All squashed per your very helpful web page there. Out of touch with the base branch because you pushed one of the CI chores, I assume I should do Update branch with the rebase option?

embano1 · 2023-01-14T07:48:02Z

@embano1 All squashed per your very helpful web page there. Out of touch with the base branch because you pushed one of the CI chores, I assume I should do Update branch with the rebase option?

Thx for the kind words!
Not sure what this UI feature does e.g., will it create a new commit? You can also do it via git (blog post has an answer too)

embano1 · 2023-01-14T07:49:42Z

Ah, there's a rebase with force-push option in that button (not default).

That's basically what you'd do with git. So pick your destiny. One challenge going through the UI is that your local git might throw errors if you needed to update that branch later. If not, do it through the UI via rebase and then throw away your local branch.

timbray · 2023-01-14T20:39:52Z

OK, I'm going to pull this on the weekend unless someone has a good reason not to.

embano1 · 2023-01-15T07:16:23Z

OK, I'm going to pull this on the weekend unless someone has a good reason not to.

You mean squash-merging this PR or pulling the v1?

embano1

LGTM

fixes: #144 fixes: #121 fixes: #81 Signed-off-by: Tim Bray <tbray@textuality.com>

embano1

LGTM

timbray requested review from yosiat and embano1 December 31, 2022 18:31

yosiat previously approved these changes Jan 1, 2023

View reviewed changes

yosiat assigned timbray Jan 1, 2023

yosiat reviewed Jan 1, 2023

View reviewed changes

cl2_test.go Outdated Show resolved Hide resolved

timbray dismissed yosiat’s stale review via 66f4f48 January 2, 2023 23:48

embano1 reviewed Jan 3, 2023

View reviewed changes

embano1 reviewed Jan 5, 2023

View reviewed changes

PATTERNS.md Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

cl2_test.go Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

timbray force-pushed the v1.0prep branch from 24ddb88 to 0b9ff15 Compare January 14, 2023 18:42

embano1 previously approved these changes Jan 16, 2023

View reviewed changes

kaizen: prepare for v1.0.0

2f9a28b

fixes: #144 fixes: #121 fixes: #81 Signed-off-by: Tim Bray <tbray@textuality.com>

timbray dismissed embano1’s stale review via 2f9a28b January 16, 2023 20:32

timbray force-pushed the v1.0prep branch from 0b9ff15 to 2f9a28b Compare January 16, 2023 20:32

embano1 approved these changes Jan 16, 2023

View reviewed changes

timbray merged commit 0b4fe1d into main Jan 17, 2023

timbray deleted the v1.0prep branch January 17, 2023 16:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kaizen: prepare for v1.0.0 #167

kaizen: prepare for v1.0.0 #167

timbray commented Dec 31, 2022

codecov-commenter commented Dec 31, 2022 •

edited

Loading

timbray commented Dec 31, 2022

timbray commented Dec 31, 2022

yosiat commented Jan 1, 2023 •

edited

Loading

yosiat Jan 1, 2023

timbray Jan 1, 2023

embano1 Jan 3, 2023

timbray commented Jan 1, 2023

embano1 Jan 3, 2023

embano1 Jan 3, 2023

timbray Jan 3, 2023

embano1 Jan 5, 2023 •

edited

Loading

timbray Jan 5, 2023

timbray commented Jan 10, 2023

embano1 commented Jan 10, 2023

timbray commented Jan 10, 2023

embano1 commented Jan 10, 2023

timbray commented Jan 10, 2023

embano1 commented Jan 10, 2023

timbray commented Jan 13, 2023

embano1 commented Jan 14, 2023

embano1 commented Jan 14, 2023

timbray commented Jan 14, 2023

embano1 commented Jan 15, 2023

embano1 left a comment

embano1 left a comment

kaizen: prepare for v1.0.0 #167

kaizen: prepare for v1.0.0 #167

Conversation

timbray commented Dec 31, 2022

codecov-commenter commented Dec 31, 2022 • edited Loading

Codecov Report

timbray commented Dec 31, 2022

timbray commented Dec 31, 2022

yosiat commented Jan 1, 2023 • edited Loading

yosiat Jan 1, 2023

Choose a reason for hiding this comment

timbray Jan 1, 2023

Choose a reason for hiding this comment

embano1 Jan 3, 2023

Choose a reason for hiding this comment

timbray commented Jan 1, 2023

embano1 Jan 3, 2023

Choose a reason for hiding this comment

embano1 Jan 3, 2023

Choose a reason for hiding this comment

timbray Jan 3, 2023

Choose a reason for hiding this comment

embano1 Jan 5, 2023 • edited Loading

Choose a reason for hiding this comment

timbray Jan 5, 2023

Choose a reason for hiding this comment

timbray commented Jan 10, 2023

embano1 commented Jan 10, 2023

timbray commented Jan 10, 2023

embano1 commented Jan 10, 2023

timbray commented Jan 10, 2023

embano1 commented Jan 10, 2023

timbray commented Jan 13, 2023

embano1 commented Jan 14, 2023

embano1 commented Jan 14, 2023

timbray commented Jan 14, 2023

embano1 commented Jan 15, 2023

embano1 left a comment

Choose a reason for hiding this comment

embano1 left a comment

Choose a reason for hiding this comment

codecov-commenter commented Dec 31, 2022 •

edited

Loading

yosiat commented Jan 1, 2023 •

edited

Loading

embano1 Jan 5, 2023 •

edited

Loading