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.

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

Maintenance status and plans #83

Open
lognaturel opened this issue Jun 3, 2021 · 7 comments
Open

Maintenance status and plans #83

lognaturel opened this issue Jun 3, 2021 · 7 comments

Comments

@lognaturel
Copy link
Collaborator

lognaturel commented Jun 3, 2021

Bonjour @albanm and other XSLT enthusiasts! Thanks for your work here.

I see from comments at #73 (comment) and #68 (comment) that @albanm may be looking to transfer maintenance and npm module ownership. @albanm would that be your ideal outcome or are you happy continuing to review and merge PRs and do releases? Or perhaps you'd like to have a co-maintainer? I ask because I know being the one person who can do merges and releases can feel like a burden.

I also ask because I contribute to a project that is currently blocked from upgrading beyond Node 12 and NPM 6 by node-libxslt. We're considering exploring a possible patch here for compatibility through Node 16 and want to get a sense of whether someone would be available to review and merge that patch (if we can do it). What would the compatibility expectations be for such a patch? At least Node 12-16? Earlier versions too?

I also wonder whether there are others who might be better positioned to make that change and/or might have plans to take over maintenance. Specifically, I see @alexdee2007 and @renanccastro did the upgrade work to Node 12.

Thanks!

@albanm
Copy link
Owner

albanm commented Jun 4, 2021

Hello,

I don't mind reviewing, merging and releasing from time to time. I don't think that offers the best service to the users as my involvement will stay very low with this project that I haven't used for a long time, but still you can be confident that if you submit a PR it will be considered.

I also don't mind giving permissions to a maintainer.

And finally if there is a fork somewhere with a more active maintainer than me I can simply create a link in the readme to recommend it as a replacement.

Honestly I am ok with all those possibilities. You the users can decide.

As for compatibility management I think a break for older node versions would not be a problem. But the PR should include a warning in the readme and the following release would be a major version.

@lognaturel
Copy link
Collaborator Author

lognaturel commented Jun 7, 2021

Thanks so much for the thoughtful response, @albanm. I think it makes sense for current users to organize somehow and look into adding and supporting a maintainer who is a current user. I appreciate your willingness to help in the mean time despite not using the library yourself anymore.

Unfortunately, my collaborators and I have pretty surface experience with C++. We'll spend some time looking at what a compatibility update would look like but unless the updates are pretty mechanical, I'd be concerned about introducing subtle bugs, memory management issues, etc.

I can't immediately tell whether libxmljs will be an issue but I think it would be. It looks like node-libxslt currently depends on https://www.npmjs.com/package/node1-libxmljsmt. The latest commit in the corresponding repo at https://github.com/gagern/libxmljs is from 2017 and I can't tell what was released a year ago as v1.0.0.

If anyone has any insights on the status of the libxmljs fork this repo depends on, it'd be much appreciated.

Side note, I also believe that libxslt v1.1.28 is being used and it's 8 years old. That doesn't necessarily matter but wanted to make a note of it as I explore the state of the repo.

@rchipka
Copy link

rchipka commented Jun 10, 2021

The main libxmljs repo is currently undergoing a massive overhaul, including support for multi-threading and Typescript support.

@lognaturel
Copy link
Collaborator Author

I tracked the libxmljs-mt 1.0.0 npm release back to gagern/libxmljs#17 by @renanccastro. Thanks for chiming in about the main repo status, @rchipka!

#68 (comment) explains the use of the libxlmljs fork. @rchipka are you aware of the concurrency issues mentioned in that thread and have you been able to include tests for them? If not, perhaps someone here could help track those cases down.

I also explored what's going on with the nan library. It seems it's likely to work with Node 16 though that will need checking (nodejs/nan#910 (comment)).

I see two possible high-level paths:

  1. look into making mechanical updates to both this repo and @renanccastro's libxmljs fork
  2. look into whether the changes to mainline libxmljs will address the issues that led to the fork and explore what it would take to switch back to it

The second seems better overall to me. However, that's not likely to be within easy reach for me given the concerns I mentioned earlier about surface C++ experience. I think I could do the first.

@lognaturel
Copy link
Collaborator Author

Thanks so much for getting #84 out, @albanm! 😍 That certainly reduces a lot of the urgency around maintenance that I was feeling.

I think I mostly got lucky this time that the changes required were minimal. I’m not really sure I can do any meaningful code review or feature work beyond that but what I can do is help keep up with addressing and closing stale issues and PRs. If you would like to give me triage access to do that, I’d be happy to help in that way.

@lognaturel lognaturel changed the title Maintenance status and plans, especially related to Node 12 end-of-life 4/2022 Maintenance status and plans Jul 16, 2021
@albanm
Copy link
Owner

albanm commented Jul 17, 2021

That's a good idea, thank you. I just sent you an invitation to become collaborator on this repository.

@KaikuOMantere
Copy link
Contributor

I was getting

no such file or directory: '/Users/omantere/xslttest2/node_modules/node1-libxmljsmt-myh/build/Release/xmljs-myh.node'

and fixed it here #94 , someone please review?

@lognaturel @albanm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants