Skip to content

Add target option#41

Merged
jhuleatt merged 9 commits intoFirebaseExtended:mainfrom
expatiating:add-target-option
Oct 23, 2020
Merged

Add target option#41
jhuleatt merged 9 commits intoFirebaseExtended:mainfrom
expatiating:add-target-option

Conversation

@expatiating
Copy link
Copy Markdown
Contributor

@expatiating expatiating commented Oct 17, 2020

Fixes issue #6

@google-cla google-cla Bot added the cla: yes Contributor has signed CLA label Oct 17, 2020
@expatiating
Copy link
Copy Markdown
Contributor Author

expatiating commented Oct 19, 2020

@jhuleatt My failing step is Error: Input required and not supplied: firebaseServiceAccount I don't believe that I manage this in my PR. Happy to help to fix, but I'm not sure if I'd have access.

@jhuleatt
Copy link
Copy Markdown
Collaborator

jhuleatt commented Oct 20, 2020 •
edited
Loading

@expatiating thank you for this PR!

@jhuleatt My failing step is Error: Input required and not supplied: firebaseServiceAccount I don't believe that I manage this in my PR. Happy to help to fix, but I'm not sure if I'd have access.

That is an access issue tracked in #17. With our current config, the action only runs successfully in this repo for PRs from owners, so no need to worry about that.

jhuleatt
jhuleatt reviewed Oct 20, 2020
View reviewed changes
Comment thread action.yml Outdated
jhuleatt
jhuleatt reviewed Oct 20, 2020
View reviewed changes
Comment thread action.yml Outdated
jhuleatt
jhuleatt reviewed Oct 20, 2020
View reviewed changes
Comment thread src/deploy.ts Outdated
jhuleatt
jhuleatt reviewed Oct 20, 2020
View reviewed changes
Comment thread src/deploy.ts Outdated
jhuleatt
jhuleatt reviewed Oct 20, 2020
View reviewed changes
Comment thread src/deploy.ts Outdated
jhuleatt
jhuleatt requested changes Oct 20, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@jhuleatt jhuleatt left a comment

Choose a reason for hiding this comment

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

Thank you @expatiating! I've added a few comments. Once those are resolved, we can get this merged!

rachelsaunders
rachelsaunders suggested changes Oct 20, 2020
View reviewed changes
Comment thread action.yml Outdated
Comment thread action.yml Outdated
expatiating and others added 5 commits October 21, 2020 23:09
@expatiating @jhuleatt
Update src/deploy.ts
1bec66e 
DRY `--only hosting` once

Co-authored-by: Jeff <[email protected]>
@expatiating @jhuleatt
Update src/deploy.ts
c8d1afc 
Co-authored-by: Jeff <[email protected]>
@expatiating @rachelsaunders
Update action.yml
2aa3beb 
Not really pertinent to my change, but reads clearer, so thank you 🤓

Co-authored-by: rachelsaunders <[email protected]>
@expatiating @rachelsaunders
Better description
b0388f8 
Co-authored-by: rachelsaunders <[email protected]>
@expatiating
Make target optional in productionDeployConfig
9f90bb8
expatiating
expatiating commented Oct 22, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor Author

@expatiating expatiating left a comment

Choose a reason for hiding this comment

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

@jhuleatt and @rachelsaunders thank you for the review.

I believe I've addressed all comments and committed all suggestions.

@expatiating expatiating requested a review from jhuleatt October 22, 2020 06:44
@jhuleatt jhuleatt merged commit 8660c48 into FirebaseExtended:main Oct 23, 2020
@jhuleatt
Copy link
Copy Markdown
Collaborator

jhuleatt commented Oct 23, 2020

Thank you @expatiating! This is now available in v0.5-alpha. You can try it with the following yml config:

- name: Deploy to Firebase Hosting
  uses: FirebaseExtended/[email protected]

I'll probably wait until early next week to release to the @v0 tag, which makes it available to everyone that set up this action via the CLI. Just want to avoid a Friday release.

@jhuleatt jhuleatt mentioned this pull request Oct 23, 2020
Closed
@expatiating expatiating deleted the add-target-option branch October 23, 2020 20:16
@expatiating
Copy link
Copy Markdown
Contributor Author

expatiating commented Oct 24, 2020 •
edited
Loading

@jhuleatt just migrated my workflow to v0.5-alpha, restored the changes I made to my firebase.json / .firebaserc to work with v0 / 79d1e17 and voila, worked like a charm!

Thank you and glad that we made this change to resolve #6! 😀

@jhuleatt jhuleatt mentioned this pull request Oct 26, 2020
Merged
pomle pushed a commit to pomle/action-hosting-deploy that referenced this pull request Nov 8, 2020
@expatiating @jhuleatt @rachelsaunders
Add target option (FirebaseExtended#41)
d967e2e 
* Add target option

* Fix format

* Update src/deploy.ts

DRY `--only hosting` once

Co-authored-by: Jeff <[email protected]>

* Update src/deploy.ts

Co-authored-by: Jeff <[email protected]>

* Update action.yml

Not really pertinent to my change, but reads clearer, so thank you 🤓

Co-authored-by: rachelsaunders <[email protected]>

* Better description

Co-authored-by: rachelsaunders <[email protected]>

* Make target optional in productionDeployConfig

* Commit prettier suggestions

* Add bin contents per FirebaseExtended#47 workflow

Co-authored-by: Jeff <[email protected]>
Co-authored-by: rachelsaunders <[email protected]>
@FirebaseExtended FirebaseExtended locked and limited conversation to collaborators Nov 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jhuleatt jhuleatt jhuleatt approved these changes

+1 more reviewer

@rachelsaunders rachelsaunders rachelsaunders requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes Contributor has signed CLA

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@expatiating @jhuleatt @rachelsaunders