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

Introduce worker-trig module #4

Closed

Conversation

StephanieJoyMills
Copy link
Owner

This adds a new module for handling MIME types. It is standalone based upon the WHATWG MIME standards, but is more intended to open up APIs that wish to use MIME for coordinating content types. I have left out the various sniffing for content bodies until a future date as I believe that is a longer discussion than just parsing and serializing. The API is based upon URL and URLSearchParams and intentionally allows us to expand MIMEParams for dealing with multiple parameters with the same name (.getAll / .append etc.).

@@ -10,7 +10,7 @@ exports.createCart = async (name, email) => {
return result[0];
};

exports.checkIfHasCart = async email => {
exports.checkIfHasCart = async (email) => {
const result = await knex("shopping_carts")
.select("id")
.where({ owner_email: email });
Copy link
Owner Author

Choose a reason for hiding this comment

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

Nit: Fits on one line

Copy link

Choose a reason for hiding this comment

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

Off the top of my head, this seems feasible, but that would make MemoryRetainer more sophisticated than what it currently is - it is now only for the heap snapshot integration, moving memory management bits there may require a bit more thoughts into this...(the general idea of making it more powerful sounds cool though, and probably can help with nodejs/diagnostics#155)

Copy link

@kaylynlau1 kaylynlau1 Apr 1, 2019

Choose a reason for hiding this comment

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

Please focus on readability for others to understand 3

Copy link
Owner Author

Choose a reason for hiding this comment

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

a

Copy link
Owner Author

Choose a reason for hiding this comment

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

1

Copy link
Owner Author

Choose a reason for hiding this comment

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

241

Copy link
Owner Author

Choose a reason for hiding this comment

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

23143

Copy link
Owner Author

Choose a reason for hiding this comment

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

32312341234

Copy link
Owner Author

Choose a reason for hiding this comment

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

1234

Copy link
Owner Author

Choose a reason for hiding this comment

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

last test 🗡

src/db-service/shopping_cart.js Show resolved Hide resolved
@StephanieJoyMills
Copy link
Owner Author

I don't plan to merge this immediately and think all the names on npm cause problems. Kind of leaning on nodejs/TSC#389 , but am also open to changing the name, all simple ones seem to conflict though.

@irkemp
Copy link

irkemp commented Apr 1, 2019

The introductory paragraph in GOVERNANCE.md does not add anything that
isn't obvious from the document itself. Remove it. a

@kaylynlau1
Copy link

kaylynlau1 commented Apr 1, 2019

Checklist
make -j4 test (UNIX), or vcbuild test (Windows) passes
documentation is changed or added
commit message follows commit guidelines
It may be easier to review the changes commit by commit, where they are grouped by a category. Last two commits are the biggest, but also easily skimmable.

Remove duplication in a description.
Fix possible typo.d
Add missing types.
Unify number/integer types.
Make notes about TypeError conditions in Buffer.from() variants more accurate.
Add a general note about integer coercion for Buffer octets.
Mark optional parameters as optional.
Add notes about negative offsets.
Unify periods in comments (except "Prints: ..." comments to avoid confusion).
Unify link formatting.1

@kaylynlau1
Copy link

The _handle property on crypto classes was runtime deprecated in node 11, this removes them.

Checklist
make -j4 test (UNIX), or vcbuild test (Windows) passes
documentation is changed or added
commit message follows commit guidelines

@StephanieJoyMills
Copy link
Owner Author

test

@StephanieJoyMills
Copy link
Owner Author

123

@StephanieJoyMills
Copy link
Owner Author

1234

@StephanieJoyMills
Copy link
Owner Author

2341

@StephanieJoyMills
Copy link
Owner Author

42

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