-
Notifications
You must be signed in to change notification settings - Fork 2
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
Added SkipFilters to SDK #86
Added SkipFilters to SDK #86
Conversation
This pull request has been linked to Shortcut Story #48140: JavaScript Decision SDK ignores some fields supplied in the request. |
… elements to use that rather than a generated folder.
- name: Tag Version | ||
run: npm --no-git-tag-version version prerelease --preid=${GITHUB_REF##*/}-${GITHUB_SHA:0:8} | ||
|
||
- name: Configure Project-Level .npmrc File | ||
run: npm config set --location=project @adzerk:registry=https://'${NPM_REGISTRY}' //'${NPM_REGISTRY}'/:_authToken='${NPM_SECRET}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like these rely on NPM_REGISTRY and NPM_SECRET, don't they? Am I right in assuming that this step won't have those values, as they've been moved into the step below instead of living at the top level?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Normally yes, but what I've done is wrap the shell variables in single quotes as the actual values will be substituted in by npm when npm publish
is run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That line creates a file called .npmrc
with the contents:
@adzerk:registry=https://${NPM_REGISTRY}
//${NPM_REGISTRY}/:_authToken=${NPM_SECRET}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
npm publish
then substitutes those values in from env variables when run
} from './generated'; | ||
RequiredError, | ||
UserdbApi | ||
} from '@adzerk/api-decision-js'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy that you removed the generated code from the repo! Much cleaner and less confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you
Added the SkipFilters parameter to the sdk.