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

Added SkipFilters to SDK #86

Conversation

honeycomb-cheesecake
Copy link
Contributor

Added the SkipFilters parameter to the sdk.

@shortcut-integration
Copy link

@honeycomb-cheesecake honeycomb-cheesecake marked this pull request as ready for review August 14, 2023 13:55
… elements to use that rather than a generated folder.
calliebensel
calliebensel previously approved these changes Aug 15, 2023
- 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}'

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?

Copy link
Contributor Author

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.

Copy link
Contributor Author

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}

Copy link
Contributor Author

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';

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you

@honeycomb-cheesecake honeycomb-cheesecake merged commit 6e89f7d into master Aug 15, 2023
5 checks passed
@honeycomb-cheesecake honeycomb-cheesecake deleted the simonramzi/sc-48140/javascript-decision-sdk-ignores-some-fields branch August 15, 2023 15:33
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.

3 participants