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

docs: setting horizon via configuration #221

Closed
guybedford opened this issue Sep 25, 2021 · 2 comments · Fixed by #224
Closed

docs: setting horizon via configuration #221

guybedford opened this issue Sep 25, 2021 · 2 comments · Fixed by #224
Assignees
@aomarks

Comments

@guybedford
Copy link

If setting horizon in configuration, it says this is not allowed. If providing --horizon along with --config it again says this is not allowed.

The docs for the configuration instead use "autoSampleConditions" which is not documented and I wasn't able to locate this in the source code either.
@guybedford guybedford mentioned this issue Sep 25, 2021
Closed
This was referenced Sep 25, 2021
Merged
Merged
@aomarks
Copy link
Member

aomarks commented Sep 25, 2021 •
edited
Loading

Sorry for the confusion. The CLI flag is --horizon, while the JSON config setting is horizons (note plural). Fixed the README in #224 (along with a bunch of other docs improvements).

The autoSampleConditions name you saw is a mistake left over when I was considering an alternate name. Which reminds me, "horizons" actually is a pretty confusing name, so I've also just sent out #225 to change the name to --auto-sample-conditions and autoSampleConditions. It will be backwards compatible with the old names.

@aomarks aomarks self-assigned this Sep 25, 2021
aomarks added a commit that referenced this issue Sep 27, 2021
@aomarks
Various README improvements (#224)
d110258 
Improved the README:
- Added a table of contents.
- New introduction example that shows comparing two benchmarks, instead of just one.
- Consolidated and moved around a few sections.
- Generally improved/made more concise some of the wording.
- Fixed one or two errors/typos.
- Fixed the test status badge.

Also threw this in:
- Separate `unit` vs `e2e` tests in `package.json`.

Fixes #221
Fixes #223
aomarks added a commit that referenced this issue Sep 27, 2021
@aomarks
Rename horizon to auto sample conditions (#225)
19fc355 
I never liked the name "horizons" (except on the day I chose it I guess). I think a much clearer name is "auto sample conditions".

This changes the `--horizon` flag and the `horizons` JSON config setting (yes... one was singular, the other plural) to `--auto-sample-conditions` and `autoSampleConditions` respectively.

It's backwards compatible, with a warning printed if you use the old style.

Related to #221
@Eldeloro1
Copy link

If setting horizon in configuration, it says this is not allowed. If providing --horizon along with --config it again says this is not allowed.

The docs for the configuration instead use "autoSampleConditions" which is not documented and I wasn't able to locate this in the source code either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aomarks aomarks

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Various README improvements google/tachometer
3 participants
@aomarks @guybedford @Eldeloro1