Skip to content

Conversation

@mrnugget
Copy link
Contributor

See this Slack thread.

Test plan

  • Ran it locally to test that it generates new files

@mrnugget
Copy link
Contributor Author

@LawnGnome testing this locally in sourcegraph/sourcegraph I found out that we'd need to delete the existing doc/cli/references/code-intel.md file for doc/cli/references/code-intel/index.md to be used by docsite. Any ideas on how to best do that? Manual delete in a PR and then re-generate docs in another PR? (Not sure how all of that fits into your release timelines for src-cli)

"extensions": &extensionsCommands,
"extsvc": &extsvcCommands,
"lsif": &lsifCommands,
"code-intel": &codeintelCommands,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way to keep supporting src lsif but hide it from -help? I'm not convinced this backwards breaking change is worth it because it punishes to customers who have gone through the effort of setting up precise intel in their CI pipelines. We only introduced src code-intel a few months ago, I'd expect the deprecation cycle to last at least 1-2 years for an enterprise product like our and I honestly think it's fine to support src lsif forever as long as it doesn't cause a maintenance burden.

Copy link
Contributor Author

@mrnugget mrnugget Aug 22, 2022

Choose a reason for hiding this comment

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

How is this backwards breaking? It changes the docs on docs.sourcgraph.com from what you see on the left to what you see on the right. It doesn't change anything about how the commands or src-cli works.

screenshot_2022-08-22_10 40 53@2x

Copy link
Contributor

@olafurpg olafurpg left a comment

Choose a reason for hiding this comment

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

LGTM 👍 I thought this PR removed the actual command itself.

@mrnugget
Copy link
Contributor Author

@LawnGnome testing this locally in sourcegraph/sourcegraph I found out that we'd need to delete the existing doc/cli/references/code-intel.md file for doc/cli/references/code-intel/index.md to be used by docsite. Any ideas on how to best do that? Manual delete in a PR and then re-generate docs in another PR? (Not sure how all of that fits into your release timelines for src-cli)

Turns out this is not an issue. I only ran into this because I didn't run the doc.go file in its entirety. That file cleans the directory so this problem doesn't show up. Still, waiting for review, Adam 😄

Copy link
Contributor

@varungandhi-src varungandhi-src left a comment

Choose a reason for hiding this comment

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

Thanks!

@mrnugget mrnugget merged commit 812cde1 into main Aug 26, 2022
@mrnugget mrnugget deleted the mrn/doc-code-intel branch August 26, 2022 08:16
scjohns pushed a commit that referenced this pull request Apr 24, 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.

5 participants