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

use local dockerfile path over git context #86

Merged

Conversation

aayushshah15
Copy link

@aayushshah15 aayushshah15 commented Dec 31, 2024

Important

Simplifies getDockerfilePath() in context.ts to use direct file paths for git contexts, removing unnecessary path operations.

  • Behavior:
    • Simplifies getDockerfilePath() in context.ts to use file paths directly when context is a git context.
    • Removes path normalization and joining logic for file paths when context is a git context.
  • Error Handling:
    • Retains file existence checks and warnings for non-file paths or missing Dockerfiles.

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

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! Reviewed everything up to df51985 in 8 seconds

More details
  • Looked at 29 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. src/context.ts:91
  • Draft comment:
    Consider storing the result of context === Context.gitContext() in a variable to avoid redundant checks.
  • Reason this comment was not posted:
    Confidence changes required: 20%
    The code is checking if the context is a git context multiple times, which is redundant. It can be optimized by storing the result in a variable.

Workflow ID: wflow_26F1WS6EQI8hFzZP


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

@aayushshah15 aayushshah15 force-pushed the 12-31-use_local_dockerfile_path_over_git_context branch from df51985 to 15092b1 Compare December 31, 2024 07:05
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 15092b1 in 9 seconds

More details
  • Looked at 29 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. src/context.ts:91
  • Draft comment:
    Consider checking if the context is a git context once and using a boolean flag to avoid redundant checks.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code checks if the context is a git context multiple times, which is redundant. It can be optimized by checking once and using a boolean flag.

Workflow ID: wflow_Bah1nwGHVdPiBfNt


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

@aayushshah15
Copy link
Author

@adityamaru Feel free to merge this if this looks good.

@aayushshah15 aayushshah15 force-pushed the 12-31-use_local_dockerfile_path_over_git_context branch from 15092b1 to 2c6265b Compare December 31, 2024 18:07
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 2c6265b in 9 seconds

More details
  • Looked at 29 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. src/context.ts:91
  • Draft comment:
    Consider storing the result of context === Context.gitContext() in a variable to avoid redundant checks.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code checks if the context is a git context multiple times, which is redundant. It can be optimized by storing the result in a variable.

Workflow ID: wflow_Lm9NeRT6Lbdg05gE


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

@aayushshah15 aayushshah15 merged commit c03b613 into master Dec 31, 2024
3 checks passed
@aayushshah15 aayushshah15 deleted the 12-31-use_local_dockerfile_path_over_git_context branch December 31, 2024 18:08
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