Skip to content

Commit

Permalink
chore: pr feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
mshanemc committed Oct 27, 2023
1 parent b0f12e2 commit 2a3dbec
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 31 deletions.
5 changes: 3 additions & 2 deletions MIGRATING_V5-V6.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ But that comes with breaking changes to to reduce the risk of "getting around" t
- AuthInfo.getFields now returns a read-only object. Use AuthInfo.update to change values in the fields.
- `setContents` method is no longer available in the ConfigFile stack. Use `update` to merge in multiple props, or `set/unset` on a single prop.
- `write` and `writeSync` method no longer accepts a param. Use other methods (`set`, `unset`, `update) to make modifications, then call write()/writeSync() to do the write.
- the use of lodash-style `get`/`set` (ex: `set('foo.bar.baz[0]', 3)`) no longer works.
- You can no longer override the `setMethod` and `getMethod`` when extending classes built on ConfigFile. Technically you could override get/set, but DON'T!
- the use of lodash-style `get`/`set`/`unset`/`unsetAll` (ex: `set('foo.bar.baz[0]', 3)`) no longer works.
- `awaitEach` is removed
- You can no longer override the `setMethod` and `getMethod` when extending classes built on ConfigFile. Technically you could override get/set, but DON'T!
- Everything related to tokens/tokenConfig is gone

## Unrelated changes that we did because it's a major version
Expand Down
18 changes: 3 additions & 15 deletions src/config/configStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ export interface ConfigStore<P extends ConfigContents = ConfigContents> {
values(): ConfigValue[];

forEach(actionFn: (key: string, value: ConfigValue) => void): void;
awaitEach(actionFn: (key: string, value: ConfigValue) => Promise<void>): Promise<void>;

// Content methods
getContents(): P;
Expand Down Expand Up @@ -188,7 +187,7 @@ export abstract class BaseConfigStore<
* Returns `true` if all elements in the config object existed and have been removed, or `false` if all the elements
* do not exist (some may have been removed). {@link BaseConfigStore.has(key)} will return false afterwards.
*
* @param keys The keys. Supports query keys like `a.b[0]`.
* @param keys The keys
*/
public unsetAll(keys: Array<Key<P>>): boolean {
return keys.map((key) => this.unset(key)).every(Boolean);
Expand Down Expand Up @@ -234,18 +233,6 @@ export abstract class BaseConfigStore<
this.entries().map((entry) => actionFn(entry[0], entry[1]));
}

/**
* Asynchronously invokes `actionFn` once for each key-value pair present in the config object.
*
* @param {function} actionFn The function `(key: string, value: ConfigValue) => Promise<void>` to be called for
* each element.
* @returns {Promise<void>}
*/
public async awaitEach(actionFn: (key: string, value: ConfigValue) => Promise<void>): Promise<void> {
const entries = this.entries();
await Promise.all(entries.map((entry) => actionFn(entry[0], entry[1])));
}

/**
* Convert the config object to a JSON object. Returns the config contents.
* Same as calling {@link ConfigStore.getContents}
Expand All @@ -266,7 +253,8 @@ export abstract class BaseConfigStore<
});
}

/** Keep ConfigFile concurrency-friendly.
/**
* Keep ConfigFile concurrency-friendly.
* Avoid using this unless you're reading the file for the first time
* and guaranteed to no be cross-saving existing contents
* */
Expand Down
4 changes: 1 addition & 3 deletions src/config/lwwMap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,7 @@ export class LWWMap<P extends ConfigContents> {

public get state(): LWWState<P> {
return Object.fromEntries(
Array.from(this.#data.entries())
// .filter(([, register]) => Boolean(register))
.map(([key, register]) => [key, register.state])
Array.from(this.#data.entries()).map(([key, register]) => [key, register.state])
) as LWWState<P>;
}

Expand Down
11 changes: 0 additions & 11 deletions test/unit/config/configStoreTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,6 @@ describe('ConfigStore', () => {
});
expect(st).to.equal('1a2b');
});
it('await each value', async () => {
const config = await TestConfig.create();
config.set('1', 'a');
config.set('2', 'b');

let st = '';
await config.awaitEach(async (key, val) => {
st += `${key}${val}`;
});
expect(st).to.equal('1a2b');
});

it('returns the object reference', async () => {
const config = new TestConfig<{ '1': { a: string } }>();
Expand Down

2 comments on commit 2a3dbec

@svc-cli-bot
Copy link
Contributor

Choose a reason for hiding this comment

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

Logger Benchmarks - ubuntu-latest

Benchmark suite Current: 2a3dbec Previous: c4ef97d Ratio
Child logger creation 328012 ops/sec (±28.49%) 558859 ops/sec (±0.45%) 1.70
Logging a string on root logger 450894 ops/sec (±11.47%) 575611 ops/sec (±13.17%) 1.28
Logging an object on root logger 293042 ops/sec (±14.47%) 320036 ops/sec (±15.25%) 1.09
Logging an object with a message on root logger 164455 ops/sec (±20.00%) 173195 ops/sec (±20.64%) 1.05
Logging an object with a redacted prop on root logger 174524 ops/sec (±21.34%) 190596 ops/sec (±21.76%) 1.09
Logging a nested 3-level object on root logger 133767 ops/sec (±22.40%) 160250 ops/sec (±21.46%) 1.20

This comment was automatically generated by workflow using github-action-benchmark.

@svc-cli-bot
Copy link
Contributor

Choose a reason for hiding this comment

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

Logger Benchmarks - windows-latest

Benchmark suite Current: 2a3dbec Previous: c4ef97d Ratio
Child logger creation 423432 ops/sec (±0.77%) 427427 ops/sec (±0.61%) 1.01
Logging a string on root logger 448849 ops/sec (±11.29%) 486759 ops/sec (±11.93%) 1.08
Logging an object on root logger 366514 ops/sec (±14.73%) 381827 ops/sec (±12.58%) 1.04
Logging an object with a message on root logger 238161 ops/sec (±15.73%) 245917 ops/sec (±15.43%) 1.03
Logging an object with a redacted prop on root logger 245625 ops/sec (±20.71%) 232089 ops/sec (±23.74%) 0.94
Logging a nested 3-level object on root logger 7291 ops/sec (±188.71%) 4919 ops/sec (±190.81%) 0.67

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.