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

feat: show meta framework name on server build success #1955

Merged

Conversation

ayoayco
Copy link
Contributor

@ayoayco ayoayco commented Nov 25, 2023

πŸ”— Linked issue

#1954

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme, or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Let me know if this is not the direction the project wants, I just assumed it is after seeing the new framework configuration :)

This PR will show the framework name and version on dev server's build success.

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@pi0
Copy link
Member

pi0 commented Nov 27, 2023

This is nice idea! Only we could show Both Nitro version and framework like Nitro Server built for XYZ v1.0.0

@ayoayco
Copy link
Contributor Author

ayoayco commented Nov 27, 2023

Alright! Will push the change in a bit :)

@ayoayco
Copy link
Contributor Author

ayoayco commented Nov 27, 2023

@pi0 Is this good or should I colorize? I pushed the change to show both Nitro & Framework info

Screenshot 2023-11-27 at 15 35 48

src/build.ts Outdated Show resolved Hide resolved
@pi0
Copy link
Member

pi0 commented Nov 27, 2023

I like the output but looking at the screenshot, seems too verbose no? We might try to colorize indeed and gray out versions or maybe just show names to be shorter. (ideas welcome to try different modes)

@pi0
Copy link
Member

pi0 commented Nov 27, 2023

On an unrelated note: If you are on Discord would you DM me please? Would love you to have you in UnJS server :)

@ayoayco
Copy link
Contributor Author

ayoayco commented Nov 27, 2023

I don't know which discord it is @pi0 😬

@ayoayco
Copy link
Contributor Author

ayoayco commented Nov 27, 2023

I like the output but looking at the screenshot, seems too verbose no? We might try to colorize indeed and gray out versions or maybe just show names to be shorter. (ideas welcome to try different modes)

I will play around more with the colors then let you know when I'm done :)

src/build.ts Outdated Show resolved Hide resolved
src/types/nitro.ts Outdated Show resolved Hide resolved
@ayoayco
Copy link
Contributor Author

ayoayco commented Nov 27, 2023

@pi0 Here's the latest screenshots...

When no framework info or showBuildSuccess is false

Screenshot 2023-11-27 at 18 28 32

When framework info & showBuildSuccess is true

Screenshot 2023-11-27 at 18 29 01

When showBuildSuccess is 'verbose'

Screenshot 2023-11-27 at 18 35 55

@pi0 pi0 modified the milestones: 2.7, 2.9 Nov 28, 2023
@pi0 pi0 changed the title feat: show framework info on dev server build success feat: show meta framework name on server build success Nov 30, 2023
@pi0
Copy link
Member

pi0 commented Nov 30, 2023

Thanks for helping on this dear @ayoayco

I have made some slight changes:

  • Option is moved and simplified to logging.devBuildSuccess
    • Version is not in the log but you can always disable the flag and use dev:reload hook for a custom message
    • We might later support template strings

@ayoayco
Copy link
Contributor Author

ayoayco commented Nov 30, 2023

All sounds good! :)

@pi0 pi0 merged commit 745630a into nitrojs:main Nov 30, 2023
4 checks passed
@pi0 pi0 mentioned this pull request Feb 27, 2024
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.

2 participants