Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove underscore #11869

Closed
wants to merge 66 commits into from
Closed

Remove underscore #11869

wants to merge 66 commits into from

Conversation

harryadel
Copy link
Contributor

@harryadel harryadel commented Jan 22, 2022

I tried to apply what we learned in meteor/blaze#331
Relevant issues: meteor/meteor-feature-requests#48
#1009 (comment)

@StorytellerCZ
Copy link
Collaborator

If we are going to merge this as one PR, then 2.6 release is a good candidate and we need to do it soon so that all changes follow from that otherwise we will spiral into endless merge resolutions.

@harryadel
Copy link
Contributor Author

@StorytellerCZ like I said, I'm down to solve any comments you guys have but meteor/blaze#359 needs to be merged to fix the tests.

@filipenevola
Copy link
Collaborator

Hi @harryadel, the tests are not passing, and also you are changing multiple packages.

I know the change is the same, but multiple PRs are necessary to get good coverage in the reviews.

As our Contribution guidelines say:

Split features up into smaller, logically separate chunks. It is unlikely that large and complicated PRs will be merged.

You can keep a single branch for your work, but please open different PRs for each package, ok?

@harryadel
Copy link
Contributor Author

@filipenevola
I'm stuck on the testing part because of meteor/blaze#359
I did a compromise on my end by undertaking this task so I thought you'd be willing to do the same and review the PR as a whole. 🤷
The changes aren't revolutionary but mostly tedious work which is something you seem to agree with.

I know the change is the same

@StorytellerCZ StorytellerCZ reopened this Apr 9, 2022
@harryadel
Copy link
Contributor Author

We have to wait for the release of 2.6 to retest things as it's failing because of the unavailability of the blaze packages.

   buf: '=> Running Meteor from a checkout -- overrides project version (Meteor 1.10.1)\n' +
        '=> Errors while initializing project:\n' +
        '\n' +
        'While selecting package versions:\n' +
        'error: No version of blaze-html-templates satisfies all constraints: @1.0.4, @=2.0.0\n' +
        'Constraints on package "blaze-html-templates":\n' +
        '* blaze-html-templates@1.0.4 <- top level\n' +
        '* blaze-html-templates@=2.0.0 <- top level\n' +
        '\n',
      fullBuffer: '=> Running Meteor from a checkout -- overrides project version (Meteor 1.10.1)\n' +
        '=> Errors while initializing project:\n' +
        '\n' +
        'While selecting package versions:\n' +
        'error: No version of blaze-html-templates satisfies all constraints: @1.0.4, @=2.0.0\n' +
        'Constraints on package "blaze-html-templates":\n' +
        '* blaze-html-templates@1.0.4 <- top level\n' +
        '* blaze-html-templates@=2.0.0 <- top level\n' +
        '\n',

@diavrank
Copy link
Contributor

Hi, have already considered updating this package meteorhacks:ssr? this package is very used to build email templates and I think it is using the underscore package.

@StorytellerCZ
Copy link
Collaborator

@diavrank that is not a core package, so it is out of scope here. You should contact the package maintainers for that.

@diavrank
Copy link
Contributor

diavrank commented Jul 4, 2022

@StorytellerCZ the thing here is that it is a unmaintained package but it is still used by many developers. Can we have a compat package for that?

@Grubba27
Copy link
Contributor

the thing here is that it is a unmaintained package but it is still used by many developers. Can we have a compat package for that?

Hey, @diavrank could you make this request an issue? so that we can discuss it and be trackable :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants