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

docs: Resolve latest SDK versions at build time #4767

Merged
merged 12 commits into from
Oct 31, 2024

Conversation

rolodato
Copy link
Member

@rolodato rolodato commented Oct 23, 2024

Thanks for submitting a PR! Please check the boxes below:

  • I have added information to docs/ if required so people know about the feature!
  • I have filled in the "Changes" section below?
  • I have filled in the "How did you test this code" section below?
  • I have used a Conventional Commit title for this Pull Request

Changes

Dynamically fetch the latest SDK versions at docs build time, instead of manually having to update the docs when any SDK version is released.

Most of the line changes in this PR are only formatting, forcefully introduced by prettier in pre-commit hooks because of the format change from .md to .mdx.

The way this works is:

  • Once, during build time, we fetch all the latest SDK versions from their corresponding package manager APIs and make them available for any doc page to consume from a Docusaurus plugin hook
  • If we fail to fetch any version during a production build, the build fails. We might want to mitigate this by adding hardcoded fallback values to avoid blocking the docs build if any remote API is unavailable
  • If we fail to fetch any version during a non-production build, we display a placeholder version of x.y.z
  • If we try to display the version for an invalid SDK, the build fails
  • Known versions are pinned to a semver specification such as ~2, which means we only need to update 1 file whenever any new major SDK version is released.

If the docs are never built, the SDK versions are never updated. This is still much better than the current situation as the docs are generally updated at a reasonable cadence, but could be further mitigated by allowing manual redeploys of the docs from a GitHub Action, or having each SDK repository trigger a docs rebuild after a new version is published.

Downsides of this approach:

  • Docusaurus does not fully support <CodeBlock> elements if the content is dynamic, so we lose out on syntax highlighting, the "copy to clipboard" button on hover, and the ability to show a title/filename header on any code blocks with dynamic content such as version numbers
  • Using < characters and sensitive whitespace within MDX code blocks is much more complicated than in plain Markdown fenced code blocks
  • We are adding a hard dependency on remote package manager APIs for the docs build. If any of these APIs fail, the docs build will fail. We might want to persist the "latest fetched versions" in this same repository, and read the known versions from that file instead.
  • It might be difficult to coordinate releasing new SDK versions and updating the corresponding docs at the same time. We'll need to become much more diligent at this if we choose this approach. This could be mitigated by "pinning" the rendered versions in docs to specific major versions, which lets us decouple new version releases from the corresponding docs changes, at the cost of more complexity in the docs build logic. solved by ed4365d

How did you test this code?

Copy link

vercel bot commented Oct 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 31, 2024 11:49pm
flagsmith-frontend-preview ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 31, 2024 11:49pm
flagsmith-frontend-staging ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 31, 2024 11:49pm

@github-actions github-actions bot added the docs Documentation updates label Oct 23, 2024
Copy link
Contributor

@matthewelwell matthewelwell left a comment

Choose a reason for hiding this comment

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

This looks great to me.

I don't think any of the downsides are strong enough for me that they outweigh the benefits here. I think we can accept the lack of syntax highlighting for the blocks that actually need the version.

@khvn26
Copy link
Member

khvn26 commented Oct 24, 2024

I'd rather invest in a centralised Docusaurus build that pulls SDK docs from their respective repos.

An ability to ship SDK documentation together with SDK code seems logical to me, and a better alternative to what we have now. I believe the documented latest version problem can be solved by this in a more precise manner, too.

IMO the SDK documentation is more valuable than the example projects that are currently maintained with SDK repos.

@rolodato
Copy link
Member Author

Pulling SDK docs from each SDK's repo is a much more intrusive change and it's more difficult to guarantee consistency across docs (things like linting, broken link/anchor checking, using Docusaurus modules if we need more than plain Markdown etc) if the content is spread out across multiple repositories. It might be desirable to eventually go there, but I still think this PR is an incremental step in the right direction (pulling some information at build from an external source).

The source of truth for latest versions is still the actual package manager APIs - whatever we pull from our repositories is not guaranteed to be actually downloadable by package managers. So even if we do pull docs from each SDK's repo, I still see a use case for pulling in the latest (relevant) SDK versions from package manager APIs. This is mostly helpful for package managers that do not have an "install latest and save to dependency manifest" command such as Maven, Gradle, Hex, Cargo, or JS CDNs.

I think a necessary condition before merging a change like this is being able to "pin" whatever we're fetching to a specific major version. That way we only need to update the docs manually whenever we release an SDK major version. I'll look into adding this before making this PR ready to review.

Also extract SDK version components to separate file
@rolodato rolodato marked this pull request as ready for review October 24, 2024 22:26
@rolodato rolodato requested a review from a team as a code owner October 24, 2024 22:26
@rolodato rolodato requested review from gagantrivedi and removed request for a team October 24, 2024 22:26
Comment on lines 27 to 36
In the new Gradle version 7+ update your `settings.gradle` file to include JitPack if you haven't already

```groovy
repositories {
google()
mavenCentral()

maven("https://jitpack.io")
}
```
Copy link
Contributor

Choose a reason for hiding this comment

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

We can't remove this yet, unless we change the below to io.flagsmith - I'm still working on finalising the deprecation of jitpack.

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed to use io.flagsmith - jitpack is more friction and can be a blocker as we've seen in the past

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm... but I'm hoping that io.flagsmith is temporary and we will (in the not too distant future, when I find time to understand the migration process for our java client) move to com.flagsmith.

I think the move from jitpack is a bigger change than just hiding it in with these changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

The com.flagsmith release is still on 1.6.1 which does not include getIdentityFlags. I'd prefer to ship with io.flagsmith even if it's temporary since it has all the latest features and customers are not blocked if they need a bugfix before we can sort out publishing to com.flagsmith. We can figure out how to publish to com.flagsmith later and it's a trivial change for consumers if they eventually want to migrate.

Copy link
Contributor

Choose a reason for hiding this comment

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

The com.flagsmith release is still on 1.6.1 which does not include getIdentityFlags

Sorry, I'm confused. The link you included isn't to the com.flagsmith (because there isn't one in maven central yet!), it's to the one where I had incorrectly included the name of the package in the namespace (see here).

To be clear, there is only the io.flagsmith releases in maven central here and the jitpack releases are just pulled straight from Github releases.

I'm not sure it's worth continuing to argue this though, we can go with what you have here if you think it's the right thing to do.

docs/src/components/SdkVersions.js Outdated Show resolved Hide resolved
@dabeeeenster
Copy link
Contributor

We could cronjob (probably weekly to maintain sanity) the docusaurus builds that pulls in SDK versions at compile time?

@dabeeeenster
Copy link
Contributor

This is great btw @rolodato ! Way above my React paygrade.

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

Successfully merging this pull request may close these issues.

4 participants