Migrating from tfussell's repo to xlnt-community

[m7913d]

To make this repo a success, it seems important to me that others also start using this repo for PR and reporting of issues.

Should we actively monitoring tfussell's repo for a while and notify them of the currently unmaintained state of tfussell's repo and of the existence of this community effort to continue his work?

@doomlaur What do you think?

[m7913d]

My suggestion is to post the following message on the now unmaintained XLNT repo of tfussell for new and recent issues and PRs:

As discussed in issue tfussell#748, this repo has been unmaintained for several years.
In order to continue this great project started by tfussell, we have created a new repo to support further development by the community.
Feel free to participate in this community effort by submitting issues and PR to this new community-driven repo.

@doomlaur What do you think?

[doomlaur]

Sorry for the delay 😅 I think it's a good idea, yes 😄 This is probably the best we can do at the moment, as we cannot change the README.md or pin an issue in the original repository.

By the way: at some point we should also think about having a documentation page, like the original XLNT documentation. For now it's not that important yet, as both repositories have the same features, but in time that will change.

[m7913d]

Also the README file should receive an update in the near future:

• describe relation between this repo and tfussell's one
• update the build/coverage status at the top of the README file to link to the actual status of our repo.

[m7913d]

I also think about hosting the API reference generated by Doxygen (using github pages?). The gitbooks seems fine for handling some important/getting started topics, but I don't like the formatting of the (incomplete) API reference in gitbooks.

[doomlaur]

Sounds very good! 👍

[m7913d]

I think most of the migration process is realized. Two things we may still consider:

1. unforking to become a stand-alone xlnt repo. In the end, I think this is the way to go, but maybe not yet. Disadvantage of being a fork is that PRs are automatically created against tfussells repo and not ours. Unforking may also make it clear that our repo exists independently of tfussells one. However, unforking has also some (short term) disadvantages: merging from forks of tfussells repo may become less trivial and we will not be listed anymore as an active fork of tfussells repo (which may make it harder to find our repo for existing xlnt users).
2. Releasing a new version / using our repo when building using vcpkg (Note that I don't have any experience with vcpkg, not even as a user)

[MyRoGeertGit]

Hello,

On https://github.com/xlnt-community/xlnt/pulls the 'New Pull Request' button refers to tfussell's version.

Thank you for your efforts.
Geert.

[doomlaur]

@m7913d You are right and have summarized the advantages and disadvantages very well. I would propose the following:

1. On my TODO list I still have to fix the issue from Exception thrown when loading an XLSX file containing defined names without localSheetId tfussell/xlnt#685, but we are now also discussing it in Update with important fixes #18, so let's see how that evolves.
2. We should also think whether any of the issues from Stability release v1.6 umbrella issue tfussell/xlnt#644 are still relevant for us - they were being considered by @tfussell as requirements for releasing XLNT 1.6.
3. Think about whether it makes sense to continue under the simple name "XLNT" or whether we should add some prefix, suffix, or and modification of the library's name. If the original XLNT would still be developed, I would definitely propose to change the name (at least slightly, using a prefix or suffix), but in our current situation, we might decide against it. I'm just not sure whether we might have any problems in the long run (e.g. with vcpkg).
4. Release a new version of the library. This is very important for many people, as many people prefer to use stable versions. XLNT 1.5 still contains many issues that have been solved, so releasing version 1.6 would be very important.
5. Update vcpkg to the newest version - especially because I saw that many people used vcpkg and did not even notice that compiling XLNT from the master branch actually was more stable than version 1.5. However, I also don't have any experience with it, neither as a maintainer, nor as a user. I guess we'll have to do some research 😉

[m7913d]

@doomlaur I think it may be a good time to create our first release:

• If you want some issues being fixed before the release. I think we should create a milestone for it. However, I would keep the number of included issues as low as possible.
• Concerning the name: I would like to keep the name as is. This would ensures vcpkg users just gets the new release. However, if vcpk doesn't accept this, I would use a name like xlnt-ce or xlnt community edition.

[doomlaur]

@m7913d I would like to do the following before the release:

1. Fix the unit tests for MSVC due to January 1st 1970 (as explained in Initial commit for supporting reading xls (old) files. #22). Done in Time improvements #27.
2. Improve the date/time functions and add them to some utils function (as explained in Initial commit for supporting reading xls (old) files. #22). Done in Time improvements #27.
3. Replace std::codecvt due to deprecation in C++17 and removal in C++26. Done in Replace std::codecvt by MultiByteToWideChar due to deprecation #29.
4. Update copyright information, as explained in Update build/install/link documentation #24. Done in Update copyright information #30.
5. Provide an improved parser for floating-point numbers that no longer requires using std::localeconv, or (for the unit tests)) std::setlocale, as explained in the discussions from Correctly copy the null terminator #2.

I'll try to get everything done today in the evening (since I'm away from home the whole day). Sorry for the delay in finishing the things listed above - I have some important things going on (see my website for more infos, if you are interested).

EDIT: I updated the progress - I'll finish the rest tomorrow morning 😉

[doomlaur]

A small update from my side, in addition to the updated comment above: number 5 from my list (improved parser, avoiding std::localeconv and std::setlocale) is mostly done (and compiles again), however, I would like to test it a bit more to make sure that it doesn't cause any issues. Such changes to serialization and deserialization can always cause issues when not being careful, and I would definitely like to avoid that. However, this change is NOT absolutely necessary for the next minor version, so if you would prefer to release it without it, I wouldn't mind either.

There's one more thing I noticed: while you updated utfcpp and I updated miniz, and while libstudxml is almost entirely up to date (apart from very few insignificant commits), the one library that is rather old is pybind11. I did not try to update it yet, but noticed that there were quite a few security reports related to it: https://security.snyk.io/vuln/?search=pybind

I'm not sure if the security reports are relevant to us in any way (I did not have time to check it out yet), but we should either update the library (if easily possible), or plan to do so very soon. Actually finding out whether the security reports are relevant is probably more work than just updating the library.

[m7913d]

If it is error prone, it may be better to keep it out of the release for now. This makes the release somewhat more stable.

Ok for updating the pybind library. However, I don't use python and I think no unit tests are available for python. So, we may need to perform some basic testing to ensure at least something works with the updated library. I don't know how important this binding is, as I would expect python having its own xlsx reader/writer.

[doomlaur]

I sent a PR for updating pybind11 in #31.

Theoretically, all changes to serialization (formatting) and deserialization (parsing) can be error prone if not tested very carefully. While I would like to add the changes, I can argue that:

1. The changes are not critical at the moment, as you already fixed some important issues in Correctly copy the null terminator #2.
2. I won't have much time over the next couple of days, so especially because of this, I would prefer to not send a rushed PR just to get it done, potentially at the risk of breaking something.
3. While you could wait for me to get it done and properly tested, I don't think the parser changes are critical for the next minor version. We can wait for me to get it done and tested, but don't have to.

One more thing before releasing a new version: I noticed that there are some last links that we did not update yet with the information of our fork:

• package.json is used, as far as I see, to release the XLNT npm package which allows others to use XLNT in the browser in WebAssembly. This file contains information that is out of date, and the two dependencies it has are also out of date.
• python/setup.py also contains information that is out of date, which is used to update the xlntpyarrow package.