| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
Review requested:
|
Sorry, something went wrong.
PR-URL: #35311 Reviewed-By: Jan Krems <jan.krems@gmail.com>
|
This landed without the required second approval. Just noting it as a polite FYI. We have a largely manual process so these things are inevitable from time to time. ¯\(ツ)/¯ One more data point that switching to the commit-queue (after it has been sufficiently real-world tested) as the primary (only?) means to land things will help us enforce a consistent process. |
Sorry, something went wrong.
|
I had it in my head that the 7 day limit had been removed in a previous PR to the guidelines. Or was the waiting period only removed for PRs with two approvals? That was my misunderstanding of that change then, apologies. |
Sorry, something went wrong.
No worries. The fact that you and 100+ other people are expected to keep the various rules straight is a bit of a process bug in my opinion. These things are simply going to happen from time to time. (Until we get a robust commit-queue that everyone is using!) |
Sorry, something went wrong.
|
Well at least we have vigilant contributors to spot these things, so thanks for letting me know about it. There's no replacing a good human eye on these things :) |
Sorry, something went wrong.
I won't stop if others feel strongly about enforcing the process here, but I'd prefer we don't revert. We have enough trouble getting stuff landed. This is a narrow doc change. If someone objects to it, they can open the revert PR or better yet open a PR with further revisions that can hopefully get consensus. |
Sorry, something went wrong.
|
agree no need to revert. |
Sorry, something went wrong.
PR-URL: #35311 Reviewed-By: Jan Krems <jan.krems@gmail.com>
PR-URL: nodejs#35311 Reviewed-By: Jan Krems <jan.krems@gmail.com>
PR-URL: nodejs#35311 Reviewed-By: Jan Krems <jan.krems@gmail.com>
| Back | FazBrowse Home | New Git URL |
This PR follows up from the discussion in nodejs/modules#556 in relaxing the constraint that "require" and "import" should be exhaustive and permitting other tools to decide to match neither of these conditions effectively.
The important property of mutual exclusivity remains though.
I also took the opportunity to clarify that "import" applies for any top-level resolve or load operation in the ES module loader and that "require" is able to resolve non-CommonJS formats, to try and make the fundamental definitions of these conditions clearer for tools etc.
It's a fine line between clarify and understandability here, another reason to work towards separating "guide" information from "api" information into two halves of this section ideally over time.
Feedback welcome.
//cc @nodejs/modules-active-members @lukastaegert @sokra.
Checklist