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

[patch] getHttpsServerOptions add params #882

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KentonYu
Copy link

@KentonYu KentonYu commented Aug 14, 2024

Change Description:
I want sign a cert for local ip, e.g. 127.1.1.1 \ 192.0.0.0, but getHttpsServerOptions can't pass the domains.

the function getHttpsServerOptions add three optional params, they are same as the function ensureCertificatesAreInstalled 's params.

  1. Do these changes impact command syntax of any of the packages? (e.g., add/remove command, add/remove a command parameter, or update required parameters)
    If Yes, briefly describe what is impacted.

  2. Do these changes impact documentation? (e.g., a tutorial on https://learn.microsoft.com/office/dev/add-ins/overview/office-add-ins)
    If Yes, briefly describe what is impacted.

If you answered yes to any of these please do the following:
> Include 'Rick-Kirkham' in the review
> Make sure the README file is correct

Validation/testing performed:

Describe manual testing done. 

@KentonYu KentonYu requested a review from a team as a code owner August 14, 2024 06:22
@KentonYu KentonYu changed the title feat: getHttpsServerOptions add params [patch] getHttpsServerOptions add params Aug 14, 2024
@akrantz
Copy link
Contributor

akrantz commented Aug 14, 2024

Seems OK to me.

@akrantz
Copy link
Contributor

akrantz commented Aug 14, 2024

It might be helpful to make it a single param "options" with the object which contains the items, so you can do getHttpServerOptions({domains: "abc"});

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