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

lib(Constants): expose constants in a more intuitive way #299

Merged
merged 1 commit into from
Aug 4, 2020

Conversation

lholmquist
Copy link
Contributor

@lholmquist lholmquist commented Aug 3, 2020

Signed-off-by: Lucas Holmquist lholmqui@redhat.com

Proposed Changes

This change exposes theConstants from the "index" so a user no longer has to path into the dist folder. I believe this was the original way to do it before the Typescript re-write

Description

* Constants can now be accessed more easily from the top level import/require

fixes cloudevents#298

Signed-off-by: Lucas Holmquist <lholmqui@redhat.com>
Copy link
Member

@lance lance left a comment

Choose a reason for hiding this comment

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

LGTM. What do you think about using Constants instead? No particular reason, I just think it looks nicer.

@lholmquist
Copy link
Contributor Author

What do you think about using Constants instead?

Not sure what you mean

@lance
Copy link
Member

lance commented Aug 3, 2020

I mean export Constants instead of CONSTANTS.

@lholmquist
Copy link
Contributor Author

I mean export Constants instead of CONSTANTS.

Yea, i like that. let me turn this into a draft so i can make the change

@lholmquist lholmquist marked this pull request as draft August 3, 2020 20:54
@lholmquist lholmquist marked this pull request as ready for review August 4, 2020 13:39
@lholmquist
Copy link
Contributor Author

I think i might just merge it like it is, using the all caps version.

@lholmquist lholmquist merged commit a7e0aa0 into cloudevents:master Aug 4, 2020
@lholmquist lholmquist mentioned this pull request Aug 7, 2020
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.

4 participants