Skip to content

Conversation

@RubenSandwich
Copy link
Contributor

No description provided.

@RubenSandwich RubenSandwich requested a review from a team as a code owner September 11, 2024 18:29
@RubenSandwich RubenSandwich requested review from rmainwork and removed request for a team September 11, 2024 18:29
@vercel
Copy link

vercel bot commented Sep 11, 2024

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

Name Status Preview Comments Updated (UTC)
web-presence-experimental-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 2, 2024 7:11pm

@RubenSandwich RubenSandwich changed the title initial implementation initial API compare tool Sep 11, 2024

module.exports = {
...require('@hashicorp/platform-cli/config/prettier.config'),
// ...require('@hashicorp/platform-cli/config/prettier.config'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we just remove this rather than commenting it out?

}

for (const [product, versions] of Object.entries(versionMetadata)) {
if (options.product && options.product !== product) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think we could use optional chaining here to avoid the need for that nullcheck?

Suggested change
if (options.product && options.product !== product) {
if (options?.product !== product) {

diffOptions
)

const outputString = `Testing API URL:\n${newApiURL}\n${difference}`
Copy link
Contributor

Choose a reason for hiding this comment

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

I might suggest os.EOL here instead of \n for clients on different OSes (Windows in particular I believe uses \r\n instead \n - though if our content authors use WSL that probably won't affect them as they'll be using a linux shell via WSL, right?).

@kaitlynnefuery
Copy link
Contributor

@RubenSandwich Is this PR safe to close?

@kaitlynnefuery kaitlynnefuery deleted the rn/api-compare branch February 18, 2025 20:48
.option('-r, --drop-keys <keys>', 'Result keys to drop', (value) =>
value.split(',').map((key) => key.trim())
)
.option('-t, --num-of-tests <number>', 'Number of tests', parseInt, 10)
Copy link
Contributor Author

@RubenSandwich RubenSandwich Mar 25, 2025

Choose a reason for hiding this comment

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

let's remove -t and just test every route in a version and product that they give us.

waypoint: 'content',
}

function getAllContentApiPaths(directory) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No longer need this because of app/api/docsPathsAllVersions.json.


const mainContentDirectory = './content'

const contentDirMap = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we don't need this as it should now be in app/utils/productConfig.mjs

'API type: "content", "nav-data", "version-metadata", or "content-versions"',
'content'
)
.option(
Copy link
Contributor Author

@RubenSandwich RubenSandwich Mar 25, 2025

Choose a reason for hiding this comment

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

-d only makes sense for the /docs api route, so leave it out for now.

'Diff type for API type "content": "metadata", "markdown", or "everything"',
'markdown'
)
.option('-r, --drop-keys <keys>', 'Result keys to drop', (value) =>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

-r only makes sense for the /docs api route, so leave it out for now.

Math.min(options.numOfTests, apiPaths.length) || 1

// let apiPathsLeft = [...apiPaths];
while (randomIndexes.length < numOfRandomIndexes) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the "fuzzy" part where it randomly tests /doc routes, I don't think this is necessary anymore.

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