Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

Adding MEAN.JS version information as part of the startup info when app loads #887

Merged
merged 2 commits into from
Sep 8, 2015

Conversation

lirantal
Copy link
Member

@lirantal lirantal commented Sep 5, 2015

I've been thinking it would be great if we can display the project version number when the app starts as that will easily allow other devs and users to clearly see which MEAN.JS release they are on.

We often get a lot of bug requests and this is usually the first version that comes up...

image

WDYT?

@lirantal lirantal self-assigned this Sep 5, 2015
@lirantal lirantal added this to the 0.4.x milestone Sep 5, 2015
@jloveland
Copy link
Contributor

👍

@codydaig
Copy link
Member

codydaig commented Sep 5, 2015

@lirantal What if someone changed the version in package.json to match their project's version? Why not have a meanjs-version attribute in package.json? That would also make things easier to managing versions in the generator and sub generators. I already have this in the proposed PR for the generator:
https://github.com/codydaig/generator-meanjs/blob/dev/0.2/app/templates/master/_package.json#L5
"meanjs-version": "0.4.0",

@lirantal
Copy link
Member Author

lirantal commented Sep 5, 2015

Sure, that's even better.
In which-case we can include both versions maybe (app and mean.js). WDYT?

@codydaig
Copy link
Member

codydaig commented Sep 5, 2015

SGTM

@mleanos
Copy link
Member

mleanos commented Sep 6, 2015

SGTM too! I like the idea of having both attributes displayed.

@lirantal
Copy link
Member Author

lirantal commented Sep 6, 2015

Great, I updated

@lirantal
Copy link
Member Author

lirantal commented Sep 6, 2015

image

@@ -2,6 +2,7 @@
"name": "meanjs",
"description": "Full-Stack JavaScript with MongoDB, Express, AngularJS, and Node.js.",
"version": "0.4.1",
"meanjs-sdversion": "0.4.1",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a typo? Shouldn't it be "meanjs-version"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, good catch I fixed it.

…isplaying both MEAN.JS project and App project version numbers on start-up
@lirantal lirantal force-pushed the feature/app-startup-version-number branch from 1f6c4f1 to a12746e Compare September 6, 2015 08:12
@lirantal
Copy link
Member Author

lirantal commented Sep 6, 2015

Fixed typo

@codydaig
Copy link
Member

codydaig commented Sep 6, 2015

LGTM.

Side note: you may want to set coveralls to allow a certain percentage of decrease because it's failing you right now.

@ilanbiala
Copy link
Member

@lirantal is this going to be part of 0.4.2?

@lirantal
Copy link
Member Author

lirantal commented Sep 6, 2015

@ilanbiala part of the next release so I guess 0.4.2. Why, is that an issue?

@lirantal
Copy link
Member Author

lirantal commented Sep 6, 2015

@codydaig I see it but it's not a real error, it's just letting you know that coverage decreased (in this case in 0.08%), it didn't fail the build (tests) and it didn't dramatically reduce code coverage (it's a known issue that we don't yet have tests for core stuff outside of the users/articles modules)

@ilanbiala ilanbiala modified the milestones: 0.4.2, 0.4.x Sep 6, 2015
@ilanbiala
Copy link
Member

@lirantal just want to make sure it's in the right milestone for when we finalize the release and check the milestone for open issues.

@lirantal
Copy link
Member Author

lirantal commented Sep 7, 2015

I updated it...

@mleanos
Copy link
Member

mleanos commented Sep 7, 2015

This LGTM

@codydaig
Copy link
Member

codydaig commented Sep 7, 2015

LGTM

@lirantal
Copy link
Member Author

lirantal commented Sep 8, 2015

Great, thanks.

lirantal added a commit that referenced this pull request Sep 8, 2015
Adding MEAN.JS version information as part of the startup info when app loads
@lirantal lirantal merged commit d1a9119 into meanjs:master Sep 8, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants