Skip to content

Conversation

@petele
Copy link
Contributor

@petele petele commented Aug 8, 2024

No description provided.

@github-actions github-actions bot added the feature definition Creating or defining new features or groups of features. label Aug 8, 2024
@petele petele requested a review from ddbeck August 13, 2024 14:59
Copy link
Collaborator

@ddbeck ddbeck left a comment

Choose a reason for hiding this comment

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

Following up on the Intl discussion elsewhere, I'm suggesting dropping the Intl-related methods in line comments.

Also: @Elchi3, you and I talked about this (and Number and some others). I think you rightly pointed out that at least some string methods ought to have their own features (e.g., at()), for symmetry with the Array features. Should we pull that stuff out now? Or would it make more sense to split it up in a separate PR after merging?

@Elchi3
Copy link
Collaborator

Elchi3 commented Aug 16, 2024

Also: @Elchi3, you and I talked about this (and Number and some others). I think you rightly pointed out that at least some string methods ought to have their own features (e.g., at()), for symmetry with the Array features. Should we pull that stuff out now? Or would it make more sense to split it up in a separate PR after merging?

I think I would pull that out now and possibly figure out in this PR what we want the "String (initial support)" feature to be here (analog to features/array.yml).

@petele
Copy link
Contributor Author

petele commented Aug 16, 2024

Based on feedback, and to somewhat mirror Array, I split string into multiple features, and create a new String group to which they all belong.

@petele petele requested review from Elchi3 and ddbeck August 16, 2024 14:35
Copy link
Collaborator

@ddbeck ddbeck left a comment

Choose a reason for hiding this comment

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

I've suggested a number of changes, mostly to names and descriptions. I didn't apply them because a few file renames are needed. Thank you!

Also, @Elchi3, optionally if you want to give this a once-over to compare to the Array features, I'd appreciate it.

@petele petele requested a review from ddbeck August 26, 2024 18:49
Copy link
Collaborator

@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

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

LGTM, great work 👍

@ddbeck ddbeck enabled auto-merge (squash) August 28, 2024 10:58
@ddbeck ddbeck merged commit 93d95db into web-platform-dx:main Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature definition Creating or defining new features or groups of features.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants