Skip to content

Refactor namespace and version setup #201

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

Merged
merged 3 commits into from
Oct 26, 2013
Merged

Conversation

marcjansen
Copy link
Member

This PR changes the following:

  • Introduce GeoExt.Version.version which contains the GeoExt version
  • Introduce GeoExt.Version.environment which will also print the versions of ExtJS and OpenLayers
  • Make GeoExt.version an alias for GeoExt.Version.version
  • Require the base class in all relevant files

Please review.

/**
* The GeoExt root object.
*
* @class GeoExt
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn' t this be @class GeoExt.version ?

@chrismayer
Copy link
Contributor

Hi @marcjansen,

thank you for this nice addition. I think this is very helpful. +1 for merging this.
I added two minor comments which are just some thoughts and could be discussed but they are not a blocker.

@marcjansen
Copy link
Member Author

Hey @chrismayer, cheers for your review. I agree with your comments and refactored the class as suggested. Would you mind having another quick glance at this?

@chrismayer
Copy link
Contributor

Thanks @marcjansen for taking the comments into consideration although they were minor ones and thanks for the ongoing effort! The PR looks very good! All tests still pass. Please merge!

marcjansen added a commit that referenced this pull request Oct 26, 2013
Refactor namespace and version setup
@marcjansen marcjansen merged commit e81f36e into geoext:master Oct 26, 2013
@marcjansen marcjansen deleted the version branch June 11, 2014 07:07
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