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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating for v9 #3

Closed
Ranieri93 opened this issue Aug 10, 2023 · 14 comments
Closed

Updating for v9 #3

Ranieri93 opened this issue Aug 10, 2023 · 14 comments

Comments

@Ranieri93
Copy link

Ranieri93 commented Aug 10, 2023

馃憢 I'm Ranieri, I stumbled upon issue #1762, and I figured that I could help!

Could you provide me some additional informations?
Like what is a suitable number of commits to measure interest? Do you have in mind a range of dates to inspect data coming from npm-stat.com?
Regarding the second point I never worked with CI configs or with Tap testing suite, but I can learn!

I'm also totally new to Open Source in general, so I hope you'll excuse me if I don't know some procedures or steps, but I'll help as much as I can!

@jsumners

@jsumners
Copy link
Member

Do you have in mind a range of dates to inspect data coming from npm-stat.com?

The default is one year to date, and I think that is sufficient. This package shows very limited usage -- https://npm-stat.com/charts.html?package=%40pinojs%2Fjson-colorizer&from=2022-08-27&to=2023-08-27

I also don't see any of the Pino packages depending on this one -- https://www.npmjs.com/package/@pinojs/json-colorizer?activeTab=dependents

From the commit history, it looks like @capaj created this fork and contributed it to the org. I'd like to hear if they remember the reasoning, and if they think it should continue to exist.

I'd also like to hear @mcollina's opinion.

My opinion is that this should be decommissioned (archived in GitHub) and marked as deprecated on npm.

@capaj
Copy link
Collaborator

capaj commented Aug 28, 2023

I changed dependency from chalk to colorette because it is faster and then moved the repo to pinojs org.

pinojs/pino-pretty#229 (comment)

if they think it should continue to exist.

I am not sure what you are talking about here. Can you specify?

@jsumners
Copy link
Member

I am not sure what you are talking about here. Can you specify?

As shown above, it is not currently used by any Pino project. Given the lack of maintenance of this package, and the lack of usage in other org projects, I think we are better off reducing our burden.

@capaj
Copy link
Collaborator

capaj commented Aug 28, 2023

Oh right. Feel free to transfer the package back to me or archive it.
I find it useful and I use it for logging on my development machine on projects I work on daily basis. I would most likely find time to add esm modules and update dependencies.

@Ranieri93
Copy link
Author

@jsumners and @capaj thank you so much! Then we'll consider this as deprecated and I'll move to the next one

@jsumners
Copy link
Member

jsumners commented Sep 4, 2023

@Ranieri93 we haven't heard @mcollina's opinion yet.

@Ranieri93 Ranieri93 reopened this Sep 4, 2023
@Ranieri93
Copy link
Author

Oh, you're entirely correct! Let's hear @mcollina!

@mcollina
Copy link
Member

mcollina commented Sep 4, 2023

It would be great if we could have colorized output of JavaScript objects in pino-pretty. However a PR to do that never materialized. I think we should ideally do that instead of archiving/transfering.

@jsumners
Copy link
Member

jsumners commented Sep 4, 2023

Okay, I have created the next branch https://github.com/pinojs/json-colorizer/tree/next

@Ranieri93
Copy link
Author

Hey @jsumners, I would like to start working on this.
Now I'll pose some questions, presumably even stupid ones. It's my first time so I'm gonna ask you for some patience. 馃槃

  • Updating dependencies: I assume try to update every package to the latest version and see if tests are still up and running. Do I need anything else?
  • Dropping Node <= 16: Here unfortunately I'm in the dark, could you elaborate a little bit?
  • Updating CI configuration: I see that here we use Travis and that the config file is pretty straight, so I guess won't be hard. Do I need to pay attention to something in particular?
  • Testing suite configuration (tap): Can you elaborate as well? Should I migrate the current framework to tap?
  • Anything else you think is required to achieve this is welcome.

Thank you so much in advance!

@jsumners
Copy link
Member

jsumners commented Sep 5, 2023

  • Updating dependencies: I assume try to update every package to the latest version and see if tests are still up and running. Do I need anything else?

No, that should be all. The only catch is to make sure we can actually use the latest versions. Some dependencies have shifted to ESM only, and that won't work. Such dependencies would need to be pinned to the latest version that is non-ESM.

  • Dropping Node <= 16: Here unfortunately I'm in the dark, could you elaborate a little bit?

This means removing the CI configuration for any version of Node less than 18. It can also mean going through the code and removing/updating any blocks that specifically tagged as something like "for Node X support".

  • Updating CI configuration: I see that here we use Travis and that the config file is pretty straight, so I guess won't be hard. Do I need to pay attention to something in particular?

I would remove Travis and migrate it to GitHub Actions.

  • Testing suite configuration (tap): Can you elaborate as well? Should I migrate the current framework to tap?

That would be nice in order to keep thing consistent across the org. But it is definitely not a hard requirement.

@Ranieri93
Copy link
Author

Should we consider this as done?

@jsumners
Copy link
Member

I think so.

@jsumners
Copy link
Member

Thank you @Ranieri93.

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

No branches or pull requests

4 participants