feat!: convert into pipeline extension

[thor]

Fixes #131.

Exports a register function allowing for this to register as a pipeline extension with Antora.

Alters the signature of generateIndex as it no longer is required together with antora-site-generator-lunr.

Requires substantially more work, although mostly in terms of:

• [x] Validating if this is a good approach
• [x] Validating how to handle options (what about environment vars?)
• [x] Updating the tests (using mocked ContentCatalog) (I've started tweaking the tests locally, but I haven't done much. If it seems applicable I thought I'd at the very least merge in the pages into the content catalogue, and alternatively see if importing the ContentCatalog from Antora could be an option if not too much scaffolding.)
• [x] Remove generateIndex as exported function

[ggrossetie]

Thanks!

It already looks good! However, I don't know if I will be able to do a thorough review since I haven't used the pipeline extension API yet.

I will try to take some time next week to play a bit more with the API.

@mojavelinux when you have time, feel free to jump into the discussion/review! 😄

Validating how to handle options (what about environment vars?)

What about the built-in mechanism to configure an extension (as described in: https://docs.antora.org/antora/3.0/extend/pipeline/configure-extension/)?

Updating the tests (consider replacing mock content catalog with real ContentCatalog)

Unless Dan strongly advises us not to do it, I would like to keep using unit tests with mocks as it allows us to test edge cases. Having said that, we can also add integration tests (I already done that in: https://github.com/Mogztter/antora-site-generator-lunr/tree/main/test).

Fixing the tests

👍🏻

Consider rewriting the signatures of generateIndex, perhaps dividing or combining things to a larger extent?

Yes we can break compatibility, this will be a major version. Speaking of which, I think we can remove module.exports = require('./generate-index').

[thor]

It already looks good! However, I don't know if I will be able to do a thorough review since I haven't used the pipeline extension API yet.

I will try to take some time next week to play a bit more with the API.

No worries! The API and its use is confined to extension.js as it currently is, with the entrypoint being the register function. 👍

Validating how to handle options (what about environment vars?)

What about the built-in mechanism to configure an extension (as described in: https://docs.antora.org/antora/3.0/extend/pipeline/configure-extension/)?

That's the one I'm using! :) Currently, I've more or less removed the environment variables in their entirety and replaced them with the config. If that sounds sufficient, then they'll stay removed.

Updating the tests (consider replacing mock content catalog with real ContentCatalog)

Unless Dan strongly advises us not to do it, I would like to keep using unit tests with mocks as it allows us to test edge cases. Having said that, we can also add integration tests (I already done that in: https://github.com/Mogztter/antora-site-generator-lunr/tree/main/test).

Alright! If so, I think the addition of getPages() to mockContentCatalog should suffice.

Consider rewriting the signatures of generateIndex, perhaps dividing or combining things to a larger extent?

Yes we can break compatibility, this will be a major version. Speaking of which, I think we can remove module.exports = require('./generate-index').

Sounds good! :+1: I'll do that and import it (and createIndexFile) from extension.js.

[mojavelinux]

Unless Dan strongly advises us not to do it, I would like to keep using unit tests with mocks as it allows us to test edge cases.

That's really up to you. What I can tell you is that we started out using a mock catalog for tests in Antora itself, and I regret it. But that's because Antora needs to test its own functionality thoroughly. For extension tests, you only care about interface, so the mock is more suitable. But there is always that slight unknown that maybe it works with the mock, but won't work with the real object. Use your best judgement and experience will guide you ;)

[mojavelinux]

Currently, I've more or less removed the environment variables in their entirety and replaced them with the config. If that sounds sufficient, then they'll stay removed.

I love this. Progress!

[mojavelinux]

I think this looks great. Definitely a huge step forward. Thanks Thor!

[mojavelinux]

As I said in the issue, we may want to merge this into a dedicated branch that will become the main branch when the project graduates.

[thor]

I've updated the pull request with the requested changes, and now I only hope I didn't miss something. :crossed_fingers: The tests run well locally, but it'd be great to get the workflow approved in order to see that the tests indeed also work in the pipeline. I have yet to do a clean project build to attempt to verify that.

WDYT @Mogztter? :)

EDIT: I can squash the commits more or completely if that's preferable; I've tried to structure them as separate changes, except for the package lock which is easier to throw at the end.

[ggrossetie]

What I plan to do:

1. Create a new branch named pipeline in this repository
2. Update this pull request to target this new branch
3. Review one last time (everything looks good but just to make sure that we didn't mess something during this migration)
4. Merge the pull request
5. Push the repository to https://gitlab.com/antora/antora-lunr-extension
   • main branch will become legacy in the new repository
   • pipeline branch will become main in the new repository

@mojavelinux @thor what do you think? I won't be able to do it this weekend (as I have other plans) but if there are no objections I will do it early next week.

[thor]

@Mogztter That sounds good to me. Let me know if you'd like a pull request (or merge request in this case) with semantic-release. It's pretty straightforward, so it'd only be to help save you some time.

Speaking of releases, would that perhaps make a 1.0.0 release for @antora/lunr-extension once moved?

[mojavelinux]

That's a good plan. Not that it matters much, but the name "extension" for the branch seems more appropriate.

Speaking of releases, would that perhaps make a 1.0.0 release for @antora/lunr-extension once moved?

I agree that starting with 1.0.0 is the right way to go. Starting with pre-1.0.0 releases just puts you in a weird spot with regards to semantic versioning. There are an infinite number of major numbers and we should make use of them ;)

[ggrossetie]

That sounds good to me. Let me know if you'd like a pull request (or merge request in this case) with semantic-release. It's pretty straightforward, so it'd only be to help save you some time.

We are not using semantic-release in the Asciidoctor/Antora community but since this project is relatively small I don't mind giving it a try. Besides, we will need to migrate to GitLab CI so we could take this opportunity to revisit the build/release process by using semantic-release.

@mojavelinux do you think this is a good idea? or do you think that, since the project will be in the @antora organization, it should use the same development/build/release workflow (or at least a similar one) as the other projects in the @antora organization?

Speaking of releases, would that perhaps make a 1.0.0 release for @antora/lunr-extension once moved?

Yes that's a good idea 👍🏻

That's a good plan. Not that it matters much, but the name "extension" for the branch seems more appropriate.

Noted 👍🏻

I agree that starting with 1.0.0 is the right way to go. Starting with pre-1.0.0 releases just puts you in a weird spot with regards to semantic versioning.

👌🏻

[mojavelinux]

@mojavelinux do you think this is a good idea?

I'm not opposed to trying new things. Personally, release libraries just don't work for me. I find myself spending more time trying to make them do what I want them to do that it would take for me to just do it. That's how I arrived at the release process we have now for Antora. But if a library can take on that work, and get it right, then I can't argue with that.

[thor]

@Mogztter Downgraded to NPM 6 after removing the ESLint packages, as well as its configuration. What do you think about approving the running of the workflow in order to see if the tests work out on Github Actions as well? Over to you! 🏓