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

adds filterExcept to normalizedHostingConfigs #3593

Merged
merged 8 commits into from
Jul 23, 2021
Merged

adds filterExcept to normalizedHostingConfigs #3593

merged 8 commits into from
Jul 23, 2021

Conversation

bkendall
Copy link
Contributor

Description

Adds support for --except when deploying to Firebase Hosting.

fixes #3397

Scenarios Tested

» firebase deploy --only hosting

=== Deploying to 'site'...

i  deploying hosting
i  hosting[site]: beginning deploy...
i  hosting[site]: found 17 files in public/
✔  hosting[site]: file upload complete
i  hosting[OTHER-SITE]: beginning deploy...
i  hosting[OTHER-SITE]: found 16 files in public/
✔  hosting[OTHER-SITE]: file upload complete
i  hosting[site]: finalizing version...
i  hosting[OTHER-SITE]: finalizing version...
✔  hosting[OTHER-SITE]: version finalized
i  hosting[OTHER-SITE]: releasing new version...
✔  hosting[site]: version finalized
i  hosting[site]: releasing new version...
✔  hosting[OTHER-SITE]: release complete
✔  hosting[site]: release complete

✔  Deploy complete!

other is the target for OTHER-SITE. I also have some functions...

» firebase deploy --except functions,hosting:other

=== Deploying to 'site'...

i  deploying hosting
i  hosting[site]: beginning deploy...
i  hosting[site]: found 17 files in public/
✔  hosting[site]: file upload complete
i  hosting[site]: finalizing version...
✔  hosting[site]: version finalized
i  hosting[site]: releasing new version...
✔  hosting[site]: release complete

✔  Deploy complete!

@bkendall bkendall requested review from mbleigh and joehan July 21, 2021 18:34
@google-cla google-cla bot added the cla: yes Manual indication that this has passed CLA. label Jul 21, 2021
@@ -56,6 +56,41 @@ function filterOnly(configs: HostingConfig[], onlyString: string): HostingConfig
return filteredConfigs;
}

function filterExcept(configs: HostingConfig[], exceptString: string): HostingConfig[] {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: exceptString seems like a redundant name since we already declare its type. Prefer just except instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In context, it's not important that it's a "string" by the name, but that it's the "option string" from the CLI options object. I'll go with exceptOption: string to make it clear (it's used quickly and briefly, so that should work).

return [];
}

const exceptSet = new Set(
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Don't love this name either - its redundant with the type, and it doesn't convey that this is only the hosting excepts. Consider something like hostingExcepts instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

exceptTargets.filter((t) => t.startsWith("hosting:")).map((t) => t.replace("hosting:", ""))
);

const configsBySite = new Map<string, HostingConfig>();
Copy link
Contributor

Choose a reason for hiding this comment

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

These are unused, no? it looks like you just add configs to these and then forget about them...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. I added the set to get rid of these and didn't get rid of them

},
];

for (const t of tests) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I've seen this pattern for testing in the backend but never in the CLI before - I'm totally stealing this :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I KNOW RIGHT?!

@bkendall bkendall requested a review from joehan July 22, 2021 23:31
@bkendall bkendall merged commit b5b6e5f into master Jul 23, 2021
@bkendall bkendall deleted the b3397-2 branch July 23, 2021 16:56
devpeerapong pushed a commit to devpeerapong/firebase-tools that referenced this pull request Dec 14, 2021
* adds filterExcept to normalizedHostingConfigs

fixes firebase#3397

* remove .only

* add changelog

* pass only'd configs to except

* one clarifying comment

* rename and cleanup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Manual indication that this has passed CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--except is ignored during hosting deployment
2 participants