Editorial: use typical phrasing for Agent Record field access #3704
Conversation
michaelficarra
left a comment
michaelficarra
left a comment
There was a problem hiding this comment.
This looks great! Thank you!
|
@sajdakabir would you please register as a contributor? |
spec.html
Outdated
| 1. Let _rawBytes_ be NumericToRawBytes(_type_, _value_, _isLittleEndian_). | ||
| 1. If IsSharedArrayBuffer(_arrayBuffer_) is *true*, then | ||
| 1. Let _execution_ be the [[CandidateExecution]] field of the surrounding agent's Agent Record. | ||
| 1. Let _AR_ be the Agent Record of the surrounding agent. |
There was a problem hiding this comment.
That's two Let _AR_ in the same abstract operation. Instead, you could have just one, before the first If.
There was a problem hiding this comment.
You could, but this also seems fine.
There was a problem hiding this comment.
Editorial Conventions says:
in algorithm steps, each alias should only be introduced (such as with "Let") at most once for any possible trace through the algorithm
and there is a trace through this algorithm that hits both Let _AR_.
There was a problem hiding this comment.
Oh I see, you're right, they're not exclusive.
|
@ljharb Am I misinterpreting the "check IPR form" GH Action?
|
|
it's still not checking the PR branch properly. If you run it locally it'll fail. It's on my list. |
|
@michaelficarra do I need to make any changes to the PR? |
|
Yes, let's combine the declarations on lines 45040 and 45044 into one step that precedes both usages, as suggested by @jmdyck. Then it should be good to go. |
i’ve made the changes accordingly. |
michaelficarra
added
the
ready to merge
michaelficarra
added
ready to merge
|
@michaelficarra i hope its fixed now |
|
This needs a rebase. Also, should it remain separate, or be squashed? |
|
@ljharb squash please |
ljharb
force-pushed
the
fix-agent-record-phrasing
branch
from
8f9457e to
e936549
Compare
ljharb
added
editorial change
request preview
github-actions
bot
removed
the
request preview
|
The rendered spec for this PR is available at https://tc39.es/ecma262/pr/3704. |
5 participants
Fixes #3702
Replaced all occurrences of 'the [[...]] field of the surrounding agent's Agent Record' with the more typical phrasing pattern used in AgentSignifier(), which first assigns the Agent Record to a variable AR and then accesses its fields.