X Tutup
Skip to content

feat(pipes): add intl (number and date) pipes#2877

Closed
piloopin wants to merge 2 commits intoangular:masterfrom
piloopin:new-pipes
Closed

feat(pipes): add intl (number and date) pipes#2877
piloopin wants to merge 2 commits intoangular:masterfrom
piloopin:new-pipes

Conversation

@piloopin
Copy link
Copy Markdown
Contributor

@piloopin piloopin commented Jul 4, 2015

Closes #2340.

@tbosch tbosch added the action: review The PR is still awaiting reviews from at least one requested reviewer label Jul 7, 2015
@vsavkin
Copy link
Copy Markdown
Contributor

vsavkin commented Jul 8, 2015

Could you split this PR into four separate PRs? The number and dates pipes are non-controversial and can be merged with a few minor changes.

The last two commits are a bit trickier. For instance, the way you use Parser to get AST nodes, which you then call eval on, will not work with Dart transformers. We need to think a little bit more about it.

@vsavkin vsavkin added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jul 8, 2015
@piloopin piloopin changed the title feat(pipes): add number/date/orderBy/filter pipes feat(pipes): add intl (number and date) pipes Jul 9, 2015
@piloopin
Copy link
Copy Markdown
Contributor Author

piloopin commented Jul 9, 2015

Thanks @vsavkin for the review. I addressed your comments.

The commits are conflicting and will require manual merge when separated. I created #2956 and #2957 for filter and orderBy pipes, but kept the other two for this PR as you said they're almost good to go.

@vsavkin vsavkin added pr_state: LGTM action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Jul 9, 2015
@vsavkin vsavkin added this to the alpha-31 milestone Jul 9, 2015
@vsavkin
Copy link
Copy Markdown
Contributor

vsavkin commented Jul 9, 2015

@piloopin Thank you. I will merge this PR today.

@tbosch tbosch closed this in b716046 Jul 9, 2015
@angular-automatic-lock-bot
Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker cla: yes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement/port standard set of pipes

4 participants

X Tutup