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

API deprecations #268

Closed
kanongil opened this issue Aug 8, 2018 · 13 comments
Closed

API deprecations #268

kanongil opened this issue Aug 8, 2018 · 13 comments
Assignees
Milestone

Comments

@kanongil
Copy link
Member

@kanongil kanongil commented Aug 8, 2018

Remove unused:

  • formatStack()
  • formatTrace()
  • callStack()
  • displayStack()
  • abort()
  • escapeJavaScript()
  • format()
  • transform()
  • isInteger() - was mapped to Number.isSafeInteger()
  • inherits() - was mapped to Util.inherits()
  • mapToObject()
  • shallow() - can be replaced with Object.assign({}, obj)
  • unique

Move to b64:

  • base64urlEncode()
  • base64urlDecode()
@kanongil kanongil added this to the 6.0 milestone Aug 8, 2018
@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented Aug 9, 2018

I'll go over the list and let you know.

@sholladay

This comment has been minimized.

Copy link

@sholladay sholladay commented Oct 1, 2018

I wonder how often the key feature of hoek.unique() is used.

For those who don't,hoek.unique(array) literally just calls Array.from(new Set(array))

@kanongil

This comment has been minimized.

Copy link
Member Author

@kanongil kanongil commented Oct 1, 2018

Yes, there are a few methods that are basically wrappers or direct alias to javascript / node features.

We might want to prune some of these as well. While they might make sense to hapi developers, when encountered in some code, the ES version should be easier to grok for new users.

@kanongil

This comment has been minimized.

Copy link
Member Author

@kanongil kanongil commented Oct 3, 2018

It would seem that some of these are used in the extended hapi universe. Eg. I found base64urlEncode() and base64urlDecode() are used in hueniverse/iron.

Further analysis shows that those are the only additional methods used in any public hueniverse/ repo.

@hueniverse hueniverse self-assigned this Oct 31, 2018
hueniverse added a commit that referenced this issue Oct 31, 2018
@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented Oct 31, 2018

Leaving once() and contain() alone for now.

@hueniverse hueniverse closed this in c1815d3 Nov 1, 2018
@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented Nov 1, 2018

I don't think we need to publish release notes for this, but you can if you want.

@nathanchapman

This comment has been minimized.

Copy link

@nathanchapman nathanchapman commented Nov 5, 2018

@hueniverse Why would there not be release notes for this? I just went to look back at the documentation for transform and found nothing.. Searched the code base and found nothing..

I finally came across this issue and found the corresponding commit. I don't see any of these functions as ever being marked @deprecated so having them evaporate with no release notes can't be good. I understand this was major versioned (correctly following semantic versioning), but the intention of major versioning something is so that someone upgrading will go look and see what changed before updating the module and breaking their application.

@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented Nov 6, 2018

hoek has always been an internal hapi utility belt. I do not treat it as a generally useful module. Following hapi policies, a breaking changes issue is clearly marked that documents the changes. We expect people using hapi modules to be familiar with this practice and when there is a new major release, to look for either release notes or breaking changes labels.

The maintainer of this module might still choose to publish release notes if he finds that useful but I think this issue is enough documentation.

@brycehendrix

This comment has been minimized.

Copy link

@brycehendrix brycehendrix commented Nov 21, 2018

I too had to hunt through the commit logs to find where the methods were removed, and wasted at least 15 minutes

@Marsup

This comment has been minimized.

Copy link
Member

@Marsup Marsup commented Nov 22, 2018

@brycehendrix If you're using hoek directly that probably means you depend on it in your project. Those methods didn't magically disappear, it was done in a breaking release. Should we conclude that you upgrade your dependencies on a major version without even looking at what changed?

@nathanchapman

This comment has been minimized.

Copy link

@nathanchapman nathanchapman commented Nov 22, 2018

@Marsup They're saying that in order to upgrade the module, they had to hunt around a series of commits, issues, etc. to find what was removed. People are using this and many other Hapi libraries (like wreck) in production code. It still blows my mind that there's no changelog in any of these projects 🤯
The only changelog I've found as part of the hapijs org is on the official website for only the hapi.js project. https://hapijs.com/updates

I'm not sure why the maintainers here are so against keeping a changelog. Wouldn't this help with internal updates across Hapi libraries as well as give transparency to your community?
And when I see stuff like:

hoek has always been an internal hapi utility belt. I do not treat it as a generally useful module.

I wonder why this was even open sourced in the first place. When you release a project, people will use it.

TL;DR: Your community matters. Release Notes / CHANGELOG matter.
https://keepachangelog.com/en/1.0.0/

@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented Nov 22, 2018

We do not keep a changelog. We properly use issues and labels. There is a limit to how much we can work to educate people about this. It has been mentioned many times before. There is a breaking changes label, there is a milestone. There is a Slack channel and a discussion list, both stacked with people who can easily answer these questions. If you can't figure out how to find those in a few clicks, that's your education gap coming from total lack of meaningful engagement with the community here.

Keeping changelog is extra effort that duplicates existing information. If someone wants to put in the effort to surface all these changes into the hapijs.com site the same way we do for core, PRs are welcomed. But complaining that other people are not doing the work for you will never fly with me.

If you don't like how the hapi.js community manages its FREE open source work, don't use it. No one owes you anything. If you want to help improve things, contribute your time or your money. This is particularly true for people working for major corporations with plenty of resources who are benefiting immensely from our work but giving back nothing.

TL;DR People who contribute nothing do not get to tell us that our community matters or how to run it. Using my free work one-sidedly does not make you part of my community.

@Marsup

This comment has been minimized.

Copy link
Member

@Marsup Marsup commented Nov 22, 2018

@nathanchapman With properly declared dependencies across the board, modules using hoek 5 methods will have it, and modules using hoek 6 won't but they're indeed okay with it. I'm still puzzled as to where is the problem, unless you or one of your modules just assumed hoek would pop out of nowhere.

@hapijs hapijs deleted a comment from nathanchapman Nov 22, 2018
@hapijs hapijs locked and limited conversation to collaborators Nov 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
6 participants
You can’t perform that action at this time.