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

V1 docs #277

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

V1 docs #277

wants to merge 2 commits into from

Conversation

pellicceama
Copy link
Collaborator

@pellicceama pellicceama commented Feb 23, 2025

Important

Comments out multiple steps in the release workflow and adds a step to push OpenAPI spec to Stainless.

  • Workflow Changes:
    • Comments out steps for getting the latest tag, calculating new version, generating changelog, fetching PR data, formatting PR data, combining changelog and PR data, extracting PR titles, summarizing PR changes with OpenAI, creating release notes, creating GitHub release, updating production branch, awaiting Vercel deployment, and running Stably Runner Action in release-workflow.yml.
    • Adds a step to push OpenAPI spec to Stainless using stainless-api/upload-openapi-spec-action@main in release-workflow.yml.

This description was created by Ellipsis for 127430d. It will automatically update as commits are pushed.

Copy link

vercel bot commented Feb 23, 2025

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

Name Status Preview Comments Updated (UTC)
openint ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 24, 2025 1:42pm

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to f528f6e in 1 minute and 29 seconds

More details
  • Looked at 32 lines of code in 1 files
  • Skipped 1 files when reviewing.
  • Skipped posting 5 drafted comments based on config settings.
1. .github/workflows/release-workflow.yml:153
  • Draft comment:
    Retaining commented-out code for creating a GitHub release may cause confusion. Consider adding a reference to a tracking issue or removing it if no longer needed.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    The code is intentionally commented out with a clear TODO marker indicating it will be re-enabled. This appears to be a temporary change during development. The comment suggests adding a tracking issue, but the TODO comment already serves as documentation of intent. Commenting on commented-out code with TODOs is generally not actionable - the author clearly knows it's commented out and plans to re-enable it.
    Perhaps leaving commented code, even with a TODO, could lead to it being forgotten. A tracking issue would make it more visible.
    The TODO is clear and this is in a workflow file that gets regular attention during releases. The commented code serves as useful documentation for when it's re-enabled.
    The comment should be removed. The commented code has a clear TODO marker and appears to be an intentional temporary change.
2. .github/workflows/release-workflow.yml:144
  • Draft comment:
    Consider pinning the stainless action to a specific version (e.g. @v1.x) instead of using @main to ensure build reproducibility.
  • Reason this comment was not posted:
    Marked as duplicate.
3. .github/workflows/release-workflow.yml:153
  • Draft comment:
    If the GitHub Release creation step is no longer needed, consider removing the commented-out code to keep the file clean.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    The code is explicitly marked as temporary with a TODO comment. The author clearly intends to re-enable this code later. Suggesting to remove it would go against the author's stated intention. The comment is not identifying a real issue that needs fixing.
    Maybe keeping commented code, even temporarily, is bad practice and should be called out?
    While commented code can be problematic, the clear TODO and temporary nature makes this an intentional change, not a cleanup oversight that needs fixing.
    The comment should be deleted as it contradicts the author's clearly stated intention to re-enable the code later.
4. .github/workflows/release-workflow.yml:151
  • Draft comment:
    Verify that the commit message 'feat(docs): updating documented docs for mintlify' aligns with project naming (openint) to avoid confusion.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    The comment is asking the PR author to verify the alignment of the commit message with project naming conventions. This falls under asking the author to confirm or verify something, which is against the rules.
5. .github/workflows/release-workflow.yml:151
  • Draft comment:
    The commit message in the 'Push spec and config to Stainless and output documented spec' step uses 'updating documented docs for mintlify', which might be more clear if rephrased for consistency (e.g. 'updating documented spec for Mintlify'). Consider revising this phrase to avoid potential confusion.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    While the suggested change might make the message slightly clearer, this is an extremely minor wording issue. The existing message is not incorrect or confusing - it's just a commit message that will be used when uploading docs. This kind of nitpicky comment about wording doesn't meet the bar of "clearly a code change required" and falls into the category of being unimportant.
    Perhaps consistency in terminology is more important than I'm giving it credit for, especially in a professional codebase. The word "spec" is more precise than "docs".
    While precise terminology is good, this is just a commit message that will be seen once and doesn't affect functionality. The current message is clear enough for its purpose.
    Delete this comment as it suggests an unimportant wording change that doesn't meet the bar of requiring a code change.

Workflow ID: wflow_krjPGiuhKlaGgYKc


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

run: |
gh release create "${{ env.new_version }}" --title "Release ${{ env.new_version }}" --notes-file release_notes.txt
- name: Push spec and config to Stainless and output documented spec
uses: stainless-api/upload-openapi-spec-action@main
Copy link

Choose a reason for hiding this comment

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

Consider pinning the version of the stainless-api/upload-openapi-spec-action instead of using @main to avoid unexpected changes. Use a specific version or commit hash.

Suggested change
uses: stainless-api/upload-openapi-spec-action@main
uses: stainless-api/upload-openapi-spec-action@v1.0.0

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 127430d in 1 minute and 42 seconds

More details
  • Looked at 285 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 6 drafted comments based on config settings.
1. .github/workflows/release-workflow.yml:28
  • Draft comment:
    Consider removing commented-out steps if they're not going to be re-enabled soon, as they add clutter and can lead to confusion.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    The presence of the TODO comment explicitly indicates these steps are meant to be re-enabled. This isn't a case of abandoned or deprecated code, but rather a temporary disabling of functionality. The comment suggesting removal could actually be harmful since these steps are intended to be restored.
    The comment raises a valid general principle about keeping code clean. Maybe there should be a timeline for re-enabling these steps?
    While code cleanliness is important, in this case the commented code serves as documentation of the intended workflow and is explicitly marked for re-enablement. Removing it would be counterproductive.
    The comment should be deleted since the commented code is intentionally disabled temporarily with plans to re-enable it, as evidenced by the TODO comment.
2. .github/workflows/release-workflow.yml:63
  • Draft comment:
    Review commented-out PR data processing steps; ensure dependencies on env vars (like 'latest_tag') are satisfied when re-enabled.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is asking the PR author to ensure that dependencies on environment variables are satisfied when re-enabling commented-out code. This falls under asking the author to ensure behavior is intended or to double-check things, which is against the rules.
3. .github/workflows/release-workflow.yml:161
  • Draft comment:
    Commented steps for production branch update and Vercel/Stably actions include TODOs; please confirm if these steps will be revisited, otherwise consider cleaning them up.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 30% <= threshold 50%
    The comment is asking the PR author to confirm if the steps will be revisited, which violates the rule against asking for confirmation of intention. It also suggests cleaning up the TODOs, which is a valid suggestion. However, the primary focus of the comment is on confirmation, which makes it non-compliant.
4. .github/workflows/release-workflow.yml:28
  • Draft comment:
    Large block of versioning and changelog generation steps has been commented out (lines 28-61). Consider removing this dead code or using conditional steps so that the file stays clean.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    The comment suggests removing "dead code", but the TODO comment and the fact that multiple related sections are commented out together suggests this is intentional temporary disabling, not dead code. The code may be re-enabled later. Making the workflow cleaner is good, but this seems premature since the commenting out appears intentional and temporary.
    The comment has a valid point about code cleanliness, but may be jumping to conclusions about this being "dead code" rather than temporarily disabled functionality.
    Given the TODO comment and systematic commenting out of related sections, this appears to be intentional temporary disabling rather than dead code that needs cleanup.
    The comment should be deleted as it mischaracterizes intentionally disabled code as dead code, and the TODO comment already indicates the intention to re-enable these steps.
5. .github/workflows/release-workflow.yml:63
  • Draft comment:
    The block for fetching PR data, formatting it with jq, extracting important lines, and summarizing with OpenAI (lines 63-142) is completely commented out. If not needed for this v1 docs release, consider removing it or clearly annotating why it’s disabled.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    The comment is technically correct in identifying commented code, but several factors make it less valuable: 1) Almost the entire workflow is commented out, not just this section 2) There's an explicit TODO comment explaining that these steps are meant to be reenabled later 3) This appears to be an intentional temporary state during a v1 docs release 4) The comment doesn't add any actionable information beyond what's already clear from the code and existing comments.
    The comment does identify a potential code cleanliness issue. Maybe there's value in tracking which parts are commented out to ensure they don't get forgotten.
    The existing TODO comment already serves the purpose of tracking the commented sections, and the temporary nature of this state is clearly documented. An additional comment would be redundant.
    The comment should be deleted as it doesn't add value beyond what's already communicated by the existing TODO comment and the clear intentional pattern of commented-out release workflow steps.
6. .github/workflows/release-workflow.yml:161
  • Draft comment:
    Production branch update and deployment steps (lines 161-182) are commented out. If these are not needed in the current release workflow, it may be better to remove them or conditionally enable them with proper documentation.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
  1. The code is already documented with a TODO comment explaining these steps will be reenabled. 2. The comment suggests either removing or documenting the code, but the code is already documented. 3. The comment doesn't provide actionable feedback beyond what's already indicated in the file. 4. The commented sections appear to be intentionally disabled as part of a larger change.
    The comment raises a valid point about code cleanliness and documentation. Maybe there's value in being more explicit about when/why these steps should be reenabled.
    The existing TODO comment already indicates these steps are meant to be reenabled, and the comment doesn't provide additional actionable value beyond that.
    Delete the comment since it doesn't provide actionable value beyond the existing TODO comment in the code.

Workflow ID: wflow_lGadDvBtRWOSmM54


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

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.

1 participant