Skip to content

Remove underscore from observe sequence#359

Merged
StorytellerCZ merged 7 commits intometeor:release-2.6from
harryadel:remove-underscore
Apr 9, 2022
Merged

Remove underscore from observe sequence#359
StorytellerCZ merged 7 commits intometeor:release-2.6from
harryadel:remove-underscore

Conversation

@harryadel
Copy link
Copy Markdown
Contributor

@harryadel harryadel commented Jan 22, 2022 •
edited
Loading

As I'm wrapping up meteor/meteor#11869 Test Group 3 is failing because it runs non-core packages and observe-sequence requires underscore which is no longer around since now it's now deprecated. There's no need for publishing a new version of Blaze or Observer-sequence; this PR just needs to be merged in dev branch so in meteor repo the latest commit can pulled. Coincidentally, I'm glad the the underscore removal was done here first as it'd not have worked if it had been done in Meteor repo first. 😅

StorytellerCZ
StorytellerCZ approved these changes Jan 23, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@StorytellerCZ StorytellerCZ left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM
We should still release a patch version of this package or include it in the next release independent of what is happening in the Meteor repo.

@harryadel harryadel mentioned this pull request Jan 25, 2022
Closed
@StorytellerCZ
Copy link
Copy Markdown
Collaborator

StorytellerCZ commented Jan 25, 2022

Let's merge this into 2.6 release and release 2.6. What do you think @filipenevola ?

fredmaiaarantes
fredmaiaarantes approved these changes Jan 26, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@fredmaiaarantes fredmaiaarantes left a comment

Choose a reason for hiding this comment

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

LGTM

renanccastro
renanccastro suggested changes Feb 2, 2022
View reviewed changes
@harryadel
Copy link
Copy Markdown
Contributor Author

harryadel commented Mar 5, 2022

Any update on this and the 2.6 release? @StorytellerCZ @renanccastro @filipenevola

@StorytellerCZ
Copy link
Copy Markdown
Collaborator

StorytellerCZ commented Apr 6, 2022

@fredmaiaarantes @jankapunkt let's include this in 2.6?

jankapunkt
jankapunkt reviewed Apr 6, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@jankapunkt jankapunkt left a comment

Choose a reason for hiding this comment

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

I would generally add it to 2.6 but I'd like to discuss one more this PR (see my comment on the code).

If this is to be a showstopper (due to further discussion) then we might publish 2.6 with this PR as-is and move the final removal of lodash (see my comment on the code) to 3.0?

@StorytellerCZ StorytellerCZ added this to the Blaze 2.6 milestone Apr 9, 2022
@StorytellerCZ
Copy link
Copy Markdown
Collaborator

StorytellerCZ commented Apr 9, 2022

@harryadel can you please address @jankapunkt comment? Then we can merge this into 2.6 and release.

@harryadel
Copy link
Copy Markdown
Contributor Author

harryadel commented Apr 9, 2022

Right away!

@StorytellerCZ StorytellerCZ changed the base branch from master to release-2.6 April 9, 2022 13:03
@StorytellerCZ
Copy link
Copy Markdown
Collaborator

StorytellerCZ commented Apr 9, 2022

@harryadel this seems to be the issue of the failing tests:

TypeError: posNew.forEach is not a function
    at diffArray

@harryadel
Copy link
Copy Markdown
Contributor Author

harryadel commented Apr 9, 2022

It's weird that some errors weren't caught by the CI before 🤔

@StorytellerCZ StorytellerCZ merged commit c8f7354 into meteor:release-2.6 Apr 9, 2022
StorytellerCZ added a commit that referenced this pull request Apr 9, 2022
@StorytellerCZ
Document #359
9ea530e
@zodern zodern mentioned this pull request Nov 14, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jankapunkt jankapunkt jankapunkt left review comments

@StorytellerCZ StorytellerCZ StorytellerCZ approved these changes

@fredmaiaarantes fredmaiaarantes fredmaiaarantes approved these changes

@filipenevola filipenevola Awaiting requested review from filipenevola

@renanccastro renanccastro Awaiting requested review from renanccastro

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Blaze 2.6

Development

Successfully merging this pull request may close these issues.

5 participants

@harryadel @StorytellerCZ @fredmaiaarantes @jankapunkt @renanccastro