Skip to content

Conversation

fredzqm
Copy link
Contributor

@fredzqm fredzqm commented May 23, 2025

Doing some refactor to make split prompt and file write actuation easier.

  1. Moved logging into config.writeProjectFile method. Audited all use cases of it and update surrounding logs to avoid duplications.
  2. Extract a confirmWriteProjectFile out of askWriteProjectFile.

It's very common in doSetup to use askWriteProjectFile.
After the refactor, we can make askQuestions calls confirmWriteProjectFile and saves the response. actuate calls askWriteProjectFile.

From Empty Folder

Screenshot 2025-05-22 at 5 57 29 PM

Re-running init again

Screenshot 2025-05-22 at 5 58 24 PM

It's arguably better than the existing behavior, which ask for confirmation even if there is no change in Firestore / storage rules.

[Existing] Re-running init

Screenshot 2025-05-22 at 5 50 14 PM

@fredzqm fredzqm requested a review from joehan May 23, 2025 00:50
Copy link
Contributor

@joehan joehan left a comment

Choose a reason for hiding this comment

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

One bug to fix, but lgtm otherwise

src/config.ts Outdated
message: "File " + clc.underline(path) + " already exists. Overwrite?",
default: !!confirmByDefault,
});
if (shouldWrite) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems reversed - shouldn't it be !shouldWrite

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks a bunch!

Missed the case when file changed in manual testing.

@fredzqm fredzqm enabled auto-merge (squash) May 23, 2025 19:44
@fredzqm fredzqm merged commit aba2118 into master May 23, 2025
48 of 50 checks passed
@fredzqm fredzqm deleted the fz/config branch May 23, 2025 19:53
@github-project-automation github-project-automation bot moved this from Approved [PR] to Done in [Cloud] Extensions + Functions May 23, 2025
TrCaM pushed a commit that referenced this pull request May 26, 2025
* split confirmWriteProjectFile out of askWriteProjectFile
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.

2 participants