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

feat(call-apply): Implemented call apply Pipes #53

Merged
merged 6 commits into from
Sep 15, 2023

Conversation

dmorosinotto
Copy link
Contributor

@dmorosinotto dmorosinotto commented Sep 15, 2023

Hi I've implemented CallPipe and ApplyPipe + added some tests and docs (hope to make it in the right places)

@nartc I edited even the CONTRIBUTING.md just to fix a little typo for the nx command to create the secondary entry because I haven't the pnx command that you wrote, or maybe it's some kind of script/alias??
And added an short section about test (because I need even the nx command to run them 😉)
Maybe another section to add is about where to put and how to write docs?

Closes #35

Copy link
Collaborator

@nartc nartc left a comment

Choose a reason for hiding this comment

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

You're right that pnx is my alias 😅

Thank you for your contribution. I have some suggestions

CONTRIBUTING.md Outdated Show resolved Hide resolved
docs/src/content/docs/utilities/call-apply.md Outdated Show resolved Hide resolved
docs/src/content/docs/utilities/call-apply.md Outdated Show resolved Hide resolved
libs/ngxtension/call-apply/src/call-apply.ts Outdated Show resolved Hide resolved
@nartc
Copy link
Collaborator

nartc commented Sep 15, 2023

@all-contributors please add @dmorosinotto for code

@allcontributors
Copy link
Contributor

@nartc

I've put up a pull request to add @dmorosinotto! 🎉

@nartc nartc changed the title Implemented call apply Pipes feat(call-apply): Implemented call apply Pipes Sep 15, 2023
@nx-cloud
Copy link

nx-cloud bot commented Sep 15, 2023

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 602c699. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


⌛ The following target is in progress

✅ Successfully ran 3 targets

Sent with 💌 from NxCloud.

@nartc
Copy link
Collaborator

nartc commented Sep 15, 2023

Looks like there's some timezone issue :D; probably better to convert back to UTC for the test?

@dmorosinotto
Copy link
Contributor Author

Looks like there's some timezone issue :D; probably better to convert back to UTC for the test?

NOOOOOOoooooooo my Date PUN totally failed for timezone issue :neckbeard: what a FAIL 👎
I'll change the expect in the test, and commit the fix, sorry for that, I was thinking that initializing Date with all the parts will be timezone safe as UTC but obviously correctly handling Timezone is NP-complete "hard problem" even worst than invalidate cache! 🤓

@nartc nartc merged commit a338920 into ngxtension:main Sep 15, 2023
7 checks passed
@dmorosinotto
Copy link
Contributor Author

@nartc we forgot to update the main README.md to point to call-apply docs
Can you fix it in a future commit? or do you prefer that I do it right now?

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.

[feat] add call - apply Pipes
2 participants