-
Notifications
You must be signed in to change notification settings - Fork 16
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.
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
bump aegir from 38.1.8 to 39.0.8 #223
bump aegir from 38.1.8 to 39.0.8 #223
Conversation
Thank you for submitting this PR!
Getting other community members to do a review would be great help too on complex PRs (you can ask in the chats/forums). If you are unsure about something, just leave us a comment.
We currently aim to provide initial feedback/triaging within two business days. Please keep an eye on any labelling actions, as these will indicate priorities and status of your contribution. |
commit: async (options) => { | ||
// @see https://github.com/ipfs/js-stores/issues/221 | ||
// eslint-disable-next-line @typescript-eslint/await-thenable, @typescript-eslint/no-confusing-void-expression |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unsure if // eslint-disable-next-line
or // @ts-expect-error
was more preferable here.
I settled on disable next line because I was able to do a more focused ignore over a blanket ignore.
@@ -108,8 +108,6 @@ export function parseShardFun (str: string): Shard { | |||
|
|||
export const readShardFun = async (path: string | Uint8Array, store: Datastore): Promise<Shard> => { | |||
const key = new Key(path).child(new Key(SHARDING_FN)) | |||
// @ts-expect-error | |||
const get = typeof store.getRaw === 'function' ? store.getRaw.bind(store) : store.get.bind(store) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like legacy code that kept getting passed forward. But, as far as I can tell, no store implements getRaw
nor is one described in the Store
interface.
@@ -28,7 +28,7 @@ describe('ShardingDatastore', () => { | |||
|
|||
it('open - empty', () => { | |||
const ms = new MemoryDatastore() | |||
// @ts-expect-error | |||
// @ts-expect-error No shard is given on purpose to generate an error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a description here.
@@ -115,7 +115,7 @@ describe('FsDatastore', () => { | |||
await fstore.open() | |||
await ShardingDatastore.create(fstore, new shard.NextToLast(2)) | |||
|
|||
const file = await fs.readFile(path.join(dir, shard.SHARDING_FN + '.data')) | |||
const file = await fs.readFile(path.join(dir, `${shard.SHARDING_FN}.data`)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why, but the linter was NOT happy with these two strings concatenating with a +
.
It was happy with a template string though.
|
||
mocks.send = sinon.replace(s3, 'send', (command) => { | ||
mocks.send = sinon.replace(s3, 'send', (command: Parameters<S3['send']>[0]) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The command that should go here, as far as I could tell, isn't exported by the s3 types. So went with a roundabout way to get the right type.
"interface-blockstore": "^5.0.0", | ||
"it-all": "^3.0.1", | ||
"it-drain": "^3.0.1", | ||
"multiformats": "^11.0.2", | ||
"uint8arrays": "^4.0.2" | ||
}, | ||
"devDependencies": { | ||
"aegir": "^39.0.8" | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved aegir to devDependencies.
export interface Blockstore <HasOptionsExtension = unknown, | ||
PutOptionsExtension = unknown, PutManyOptionsExtension = unknown, | ||
GetOptionsExtension = unknown, GetManyOptionsExtension = unknown, GetAllOptionsExtension = unknown, | ||
DeleteOptionsExtension = unknown, DeleteManyOptionsExtension = unknown> extends Store<CID, Uint8Array, Pair, HasOptionsExtension, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{}
wasn't well liked by the linter. Thought maybe Record<string, unknown
would be better. Eventually settled on unknown to solve the linter's complaints but would love a more specific type if someone knows one that is better.
"interface-datastore": "^8.0.0", | ||
"iso-random-stream": "^2.0.0", | ||
"it-all": "^3.0.1", | ||
"it-drain": "^3.0.1", | ||
"it-length": "^3.0.1", | ||
"uint8arrays": "^4.0.2" | ||
}, | ||
"devDependencies": { | ||
"aegir": "^39.0.8" | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved aegir over as a devDependency
"dependencies": { | ||
"aegir": "^38.1.0" | ||
"devDependencies": { | ||
"aegir": "^39.0.8" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved aegir over as a devDependency
fcd2816
to
57d4268
Compare
Ah, thanks so much for picking this up, I'm really sorry I missed it though & the aegir upgrade was done in #225 Thanks for opening this though! |
This PR is the culmination of #221 in an effort to upgrade js-stores to use the latest version of aegir.
Dependabot was unable to update to the latest version of aegir due to linting issues. These are similar to the issues that helia ran into a few weeks ago (see: ipfs/helia#111).
This PR namely bumps aegir's version and fixes the resulting linting errors that followed.