Skip to content

chore(deployment): Exclude generated .env file under presto-clp from VCS tracking (fixes #1422).#1499

Merged
junhaoliao merged 4 commits into
y-scope:mainfrom
junhaoliao:ignore-dotenv
Oct 26, 2025
Merged

chore(deployment): Exclude generated .env file under presto-clp from VCS tracking (fixes #1422).#1499
junhaoliao merged 4 commits into
y-scope:mainfrom
junhaoliao:ignore-dotenv

Conversation

@junhaoliao

@junhaoliao junhaoliao commented Oct 25, 2025 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

Description

As the title says.

Checklist

  • The PR satisfies the contribution guidelines.
  • This is a breaking change and that has been indicated in the PR title, OR this isn't a
    breaking change.
  • Necessary docs have been updated, OR no docs need to be updated.

Validation performed

  1. Followed instructions at https://github.com/y-scope/clp/blob/b20b37d37786b7d446b5f0066cd833477f5963ff/docs/src/user-docs/guides-using-presto.md to run scripts/set-up-config.sh /path/to/build/clp-package, which generated .env in tools/deployment/presto-clp/.
  2. Ran git status and found the .env file was not reported by the "Untraced files" section.

Summary by CodeRabbit

  • Chores
    • Added a header to the ignore configuration and added a rule to exclude the environment file from version control, with no impact to functionality or user experience.

@junhaoliao junhaoliao requested a review from a team as a code owner October 25, 2025 08:48
@coderabbitai

coderabbitai Bot commented Oct 25, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Walkthrough

Adds a header comment and a new ignore entry for /.env to tools/deployment/presto-clp/.gitignore. Existing ignore for /worker/config-template/clp.properties remains present; no code changes.

Changes

Cohort / File(s) Change Summary
Gitignore Configuration
tools/deployment/presto-clp/.gitignore		 Added header comment # Generated config files and an entry to ignore /.env; existing /worker/config-template/clp.properties entry was already present and unchanged.

Sequence Diagram(s)

Not applicable — no control-flow or feature changes.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related issues

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "chore(deployment): Exclude generated .env file under presto-clp from VCS tracking (fixes #1422)" directly and accurately reflects the main change in the changeset, which adds the /.env entry to the .gitignore file in tools/deployment/presto-clp/. The title is specific and clear about both what was changed (excluding the .env file) and where (under presto-clp), making it easy for a teammate scanning the history to understand the primary objective. The title follows Conventional Commits style and is concise without unnecessary noise, noise, or overly broad language.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4b6955c and 52668fc.

📒 Files selected for processing (1)
  • tools/deployment/presto-clp/.gitignore (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: package-image
🔇 Additional comments (1)
tools/deployment/presto-clp/.gitignore (1)

1-2: Changes look good — aligns with previous feedback.

The .env ignore entry (with leading /) and the descriptive comment header match kirkrodrigues' suggestions from the earlier review. The changes directly address the PR objective by excluding the generated .env file from version control. Existing ignore rules are preserved unchanged.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

junhaoliao
junhaoliao commented Oct 25, 2025
View reviewed changes
Comment thread tools/deployment/presto-clp/.gitignore Outdated
junhaoliao
junhaoliao commented Oct 25, 2025
View reviewed changes
Comment thread tools/deployment/presto-clp/.gitignore

@junhaoliao junhaoliao Oct 25, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

we should also avoid modification of the VCS tracked coordinator/config-template/split-filter.json, maybe in a separate PR?

@kirkrodrigues kirkrodrigues Oct 26, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Right, we can do it in a separate PR. That said, I'm not sure how we'd avoid modification of it unless we have a template file and then "generate" it by copying it if it doesn't exist?

@junhaoliao junhaoliao Oct 26, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

copying makes sense

i'll probably go straight to merge the docker compose projects, after the release, so we don't have to do more hacks / mitigations

@junhaoliao

junhaoliao commented Oct 25, 2025

Copy link
Copy Markdown
Member Author

@codex review this

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 25, 2025

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@junhaoliao

junhaoliao commented Oct 25, 2025

Copy link
Copy Markdown
Member Author

If Codex has suggestions, it will comment; otherwise it will react with 👍.

cool

junhaoliao and others added 2 commits October 26, 2025 14:19
@junhaoliao @kirkrodrigues
Update .gitignore - apply suggestions
5a3b727 
Co-authored-by: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com>
@junhaoliao
Merge branch 'main' into ignore-dotenv
52668fc
@junhaoliao junhaoliao requested review from kirkrodrigues and removed request for kirkrodrigues October 26, 2025 18:20
@junhaoliao junhaoliao merged commit 9e82bdb into y-scope:main Oct 26, 2025
20 checks passed
@junhaoliao junhaoliao deleted the ignore-dotenv branch October 26, 2025 18:23
junhaoliao added a commit to junhaoliao/clp that referenced this pull request May 17, 2026
@junhaoliao @kirkrodrigues
chore(deployment): Exclude generated .env file under presto-clp f…
b62a9cb 
…rom VCS tracking (fixes y-scope#1422). (y-scope#1499)

Co-authored-by: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kirkrodrigues kirkrodrigues kirkrodrigues approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@junhaoliao @kirkrodrigues