Skip to content

Conversation

@osfameron
Copy link
Collaborator

@osfameron osfameron requested a review from a team June 6, 2023 07:43
@osfameron osfameron changed the title AV-55075 update tab striping AV-55075 Update playbook and Jenkins for @asciidoctor/tabs (Prod) Jun 6, 2023
@osfameron osfameron requested a review from leeming June 6, 2023 12:31
@osfameron
Copy link
Collaborator Author

This is for Prod.
See #696 for staging

"@antora/cli": "~3.0",
"@antora/site-generator-default": "~3.0",
"@antora/site-generator-ms": "git+https://gitlab.com/opendevise/oss/antora-site-generator-ms#as-extension",
"@asciidoctor/tabs": "^1.0.0-beta.5",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess my only thing - are we cool with implementing something that's still in beta and have we tested appropriately?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question! @mojavelinux what remains for you to call this a 1.0.0 release?
Happy to put some resources into testing with a specific plan against our estate.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, I was kind of waiting for Couchbase to migrate to Asciidoctor Tabs as that could tease out any bugs that may have been introduced by setting up the project. If you can verify that it's ready to go, that would definitely give me the verification I need to call it 1.0.0.

Keep in mind that beta means "no breaking changes, only bug fixes and new features". So a beta is very stable. It's only really an alpha I would question. But still, we might as well get it over the line.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mojavelinux, I wondered if that might be the case!
From my POV, the docs were very clear and smooth. The main things I noted in the upgrade process were:

  • the preview extension loading, which you submitted the fix for (but that's just tech debt on our part)
  • overriding the CSS. (I ended up deleting all our existing CSS, and copying/customizing the bundled @asciidoctor/tabs.css if you have any comments on AV-55075 Tabset upgrade docs-ui#165 do shout). But again, I imagine most/all users will be incorporating this fresh, rather than upgrading from the original script.
  • the repo README.adoc should link directly to the excellent Antora page.

@sarahlwelton can you have a quick look at your sections for any issues? (and let me know your thoughts on what additional testing will reassure you best)

idprefix: '@'
idseparator: '-@'
tabs: tabs
tabs-sync-option: true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The correct way to turn on an attribute (in this case global option) is an empty string, not the value "true". In other words:

tabs-sync-option: ''

Copy link
Collaborator Author

@osfameron osfameron Jun 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I've seen that, but as an empty string "looks" falsey, and it looks like any defined value counts as truthy, this felt more explicit?

Is the fact that true works undefined behaviour, and therefore
likely to change in future?

Copy link
Contributor

@mojavelinux mojavelinux Jun 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may look weird, but it is how AsciiDoc has always been. The reason for it is that in a document, we can't rely on types like true and false. (The value true would just mean the attribute has the string value "true"). So it's better to be consistent with what is available there. It's the equivalent to:

:tabs-sync-option:

In the AsciiDoc Language project, we may eventually define a true value as a way to officially set and attribute (to an empty value)...but we haven't crossed that bridge. However, we can be certain that an empty value will always set the attribute.

package.json Outdated
"@antora/cli": "~3.0",
"@antora/site-generator-default": "~3.0",
"@antora/site-generator-ms": "git+https://gitlab.com/opendevise/oss/antora-site-generator-ms#as-extension",
"@asciidoctor/tabs": "^1.0.0-beta.5",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be best to depend on the exact version whenever using a prerelease. So 1.0.0-beta-5 (no leading caret).

@osfameron
Copy link
Collaborator Author

Tab sync is looking good so far on Staging.
Have submitted couchbaselabs/docs-devex#53 on docs-devex as a trial of the kind of (quite minimal) change we might want to make in a few places to improve the user experience further.

Once merged, we can keep an eye on it in Prod and report back to @mojavelinux with any issues before flicking switch to stable 1.0

@osfameron osfameron merged commit 9f610ec into couchbase:master Jun 16, 2023
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

Successfully merging this pull request may close these issues.

3 participants