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

chore!: drop support for node 12-14 #1615

Merged
merged 1 commit into from
Jun 27, 2023
Merged

chore!: drop support for node 12-14 #1615

merged 1 commit into from
Jun 27, 2023

Conversation

robertsLando
Copy link
Member

@robertsLando robertsLando commented Jun 27, 2023

BREAKING CHANGE: Dropped support for NodeJS 12-14

BREAKING CHANGE: Droppend support for NodeJS 12 and 14
@codecov
Copy link

codecov bot commented Jun 27, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (3a80bde) 86.22% compared to head (7e97c2f) 86.22%.

❗ Current head 7e97c2f differs from pull request most recent head 83096eb. Consider uploading reports for the commit 83096eb to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1615   +/-   ##
=======================================
  Coverage   86.22%   86.22%           
=======================================
  Files          13       13           
  Lines        1314     1314           
=======================================
  Hits         1133     1133           
  Misses        181      181           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@robertsLando
Copy link
Member Author

Is there anything else I should do to drop the support?

@vishnureddy17
Copy link
Member

Pretty sure that's it.

@robertsLando robertsLando removed the request for review from BertKleewein June 27, 2023 12:48
@robertsLando robertsLando merged commit a2cbf61 into main Jun 27, 2023
4 checks passed
@robertsLando robertsLando deleted the drop-old-node branch June 27, 2023 12:48
@vishnureddy17
Copy link
Member

vishnureddy17 commented Jun 27, 2023

@robertsLando Since this is a breaking change, I think we should take this opportunity to see if there's any other breaking changes we want to make before the next release. One thing I have in mind is removing the WeChat specific stuff, I do not think it belongs in this.

@robertsLando
Copy link
Member Author

One thing I have in mind is removing the WeChat specific stuff, I do not think it belongs in this.

I will give a check to that, what about creating a beta for now? Just to check if the release process works

@vishnureddy17
Copy link
Member

Yep that sounds good.

@robertsLando
Copy link
Member Author

Should I remove the wxs and alis protocols so ?

@vishnureddy17
Copy link
Member

Yep, but it doesn't have to be in this test beta release unless you want to do it now :)

I think there are references to it in the Readme that would have to be removed also

@robertsLando
Copy link
Member Author

robertsLando commented Jun 27, 2023

Yeah I'm checking them now there are many references around the code. What's the reason you want to drop that support?

@vishnureddy17
Copy link
Member

It's not a standard transport protocol and it's instead specific to a proprietary product. I don't think that belongs in an open source project.

Yeah I'm checking them now there are many references around the code.

Yeah, that's the problem, the proprietary stuff has seeped into lots of areas in the code, which I think is a mistake.

However, this library doesn't have support for custom transports, so maybe we have to tolerate this or add support for custom transport.

@robertsLando
Copy link
Member Author

However, this library doesn't have support for custom transports, so maybe we have to tolerate this or add support for custom transport.

Yeah that's what I wanted to point out, I mean without a custom transport support all users using that protocol would have no alternatives

@robertsLando
Copy link
Member Author

robertsLando commented Jun 27, 2023

@robertsLando
Copy link
Member Author

@vishnureddy17 If that is ok I would wait to merge some other PR marked with good-to-merge label and then make the official release. How can we inform users to try using the beta ?

@vishnureddy17
Copy link
Member

Yes, I agree with waiting on those PRs, there is no rush

To inform users I think replying to this GitHub issue would be a good idea, since there might be interested parties that are subscribed to that issue. Also maybe add a temporary announcement somewhere near the top of the README.

@BearCooder
Copy link

Hey @robertsLando thank you for the dedication and work! Its great to see that you picked it up and try to manage/maintain mqtt.js!

I want just make a suggestion. Have you think about Github Sponsors for MQTT.js library? https://github.com/sponsors

Just to make it sustainable, the time and effort of the team to contribute to this library. I believe MQTT.js should setup github sponsors so it can accept donations. After all time is precious and I remember this tweet from Matteo Collina - https://twitter.com/matteocollina/status/1083322337292890112?lang=en
Open Source must be sustainable :)

And I would suggest to also add the beta release note at the top of the README (temporarily) So its the first that users see that a v5 beta to test is available when they open the repository and scroll to the readme

grafik

@robertsLando
Copy link
Member Author

robertsLando commented Jun 27, 2023

Hi @BearCooder,

I already use GH sponsors in all my projects and I think that would be a good addition for sure. Anyway the project isn't mine so I would like to know @vishnureddy17 opinion on this... how could we manage this? We could enable GH sponsor in the organization but then I dunno who could maintain that

@vishnureddy17
Copy link
Member

Couple of thoughts:

  1. This isn't my project either necessarily, you and I are on equal footing :)
  2. Where would the money go to? My contributions to this are on behalf of my employer so I shouldn't be getting any sponsorships. I think a better way to do it would be to add links to contributors such as @robertsLando who would like to get sponsors. Maybe a column in this part of the readme with links to sponsor individual contributors?
    image

@robertsLando
Copy link
Member Author

Maybe a column in this part of the readme with links to sponsor individual contributors?

Maybe adding a ## Sponsor section to readme could make it more visible

@robertsLando
Copy link
Member Author

I created #1617

Let me know your thoughts about that and if I should add someone to sponsors

robertsLando added a commit that referenced this pull request Jun 30, 2023
BREAKING CHANGE: Dropped support for NodeJS 12-14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants