Closed DustinBrett closed 1 year ago
@DustinBrett The FileSystemAccess
backend does not implement Cred
. I'm not sure why it was merged if there were build errors.
I can take a look at updating FileSystemAccess
to correctly implement Cred
.
Ya I don't think the builds have passed in a while, doesn't seem to stop merges. I half put my PR up to get a discussion going about if anything needed changing, but it's been merged with quite a few others. I will also look at if there is anything I can PR a fix for. As it is I use a custom build anyway, but I was following this repo in my package.json for typing purposes and I've had to lock that to an older commit until this is resolved.
I can see about backing out your PR to see if it helps. Which one was it?
I can see about backing out your PR to see if it helps. Which one was it?
Thanks for the offer but perhaps we can just walk back to before build errors. My PR at one point was fine I think, can we step back Cred or merge a PR to add support for this new feature to all current backends?
I just merged a PR by @james-pre that adds Cred
so please see if that helps!
@emeryberger I was just about to mention that...
On the topic of building, I think we should take a look at updating the CI/CD setup. I use Azure Pipelines for a few projects and think it could work really well (plus reducing all the packages for Karma and Travis CI)
Updating the CI setup would be great - I will happily merge them. I'd love to get us back to passing the CI tests.
@DustinBrett any news?
I'm not sure about that change to Cred
which looks to have changed the API in general. Hopefully that isn't the case as the callback being the 2nd argument is something in most codebases I'd imagine already.
I haven't had time to look into much of the issues since this merge. I think maybe too much got merged in general. Feel free to disable my backend until all the build issues are fixed, but if the API has changed because of Cred
, I will likely stop upgrading for a while until I see a reason to.
@DustinBrett With Cred, only the internal BFS API has changed. Externally, it still behaves just like Node's. In addition, if you want to use a different uid/gid for the external BFS API you can do something like this:
BrowserFS.FileSystem.InMemory.Create((err, inMem) => {
BrowserFS.initalize(inMem, uid, gid);
});
const fs = BrowserFS.BFSRequire('fs');
Edit: Also, if you want I can open a PR to add uid/gid as an option to configure
/configureAsync
.
That's good to hear. Sorry I haven't dug into it further. I typically use a custom build so I haven't followed the repo too much other than using it for typings. Hopefully that latest PR helped with build issues, but the health of the project in general needs fixing probably and then the CI being green and no more merges unless it stays green.
@DustinBrett This should be resolved since #345 and #347 got merged. Please reach out if it isn't.
Now when I run
yarn upgrade
on BFS it is causing build errors. I see some of them are related to things from my PR merging, but not all. Some such asCred
issues are from the latest PR by @james-pre. I'm not sure if this is because of other PR's being merged not liking what my PR had or what, because locally my PR was building, but now not.