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

Implements GNIP #3718 (Worldmap contrib application) #3817

Merged
merged 151 commits into from
Jun 5, 2018

Conversation

capooti
Copy link
Member

@capooti capooti commented May 29, 2018

This PR adds the GeoNode contrib application in GeoNode.
It also include a few enhancements (like for example using u18n in a number of places)

JorgeMartinezG and others added 30 commits September 8, 2017 10:18
Removing certification app from GeoNode
[WIP] Dataverse and datatables migration
@afabiani
Copy link
Member

Hi @capooti great work thanks. Will test the PR ASAP on my side.
I see also a lot of effort on improving translations, very nice.

Just few comments:

  1. In theory we should first commit the PR into unstable (master) and then backport to stable (2.7.x/2.8.1). Nevertheless I see that this is mostly a contrib module, so it should not affect that much the core, but I guess we need to ask the community first before including this into the 2.8.1 release. I would not break people instances. What do you think? And also, how difficult would be to port this stuff on Django 1.11?

  2. Thinking out loud here, not really proposing solution... everytime we introduce contrib modules, we are somehow forced to put settings and checks on core also. This is not quite good in general architecturally speaking. We should envisage a way to really have contrib apps fully pluggable (except for real improvements/fixes/extensions to the core that are useful in general)

  3. In order to fix pep8 issues, you need to do a "pip install -e . --upgrade" first. This will update flake8 and pyflakes versions.

@@ -92,5 +93,6 @@ class Meta:
'username',
'is_staff',
'is_superuser',
'is_active',
'date_joined')
'is_active'
Copy link
Member

Choose a reason for hiding this comment

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

I don't quite understand why you removed a comma here. It looks to me a syntax error.

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 for spotting it! I am not sure how this happened.

@codecov
Copy link

codecov bot commented May 30, 2018

Codecov Report

Merging #3817 into 2.7.x will decrease coverage by 0.04%.
The diff coverage is 42.22%.

Impacted file tree graph

@@            Coverage Diff             @@
##            2.7.x    #3817      +/-   ##
==========================================
- Coverage   42.61%   42.56%   -0.05%     
==========================================
  Files         257      261       +4     
  Lines       18092    18166      +74     
  Branches     2753     2760       +7     
==========================================
+ Hits         7710     7733      +23     
- Misses       9850     9898      +48     
- Partials      532      535       +3
Impacted Files Coverage Δ
geonode/groups/forms.py 46% <ø> (ø) ⬆️
geonode/people/forms.py 82.85% <ø> (ø) ⬆️
geonode/people/models.py 85.52% <ø> (ø) ⬆️
geonode/context_processors.py 40.9% <0%> (-59.1%) ⬇️
geonode/maps/migrations/0026_map_content_map.py 100% <100%> (ø)
geonode/layers/models.py 74.11% <100%> (ø) ⬆️
geonode/maps/migrations/0030_merge.py 100% <100%> (ø)
geonode/groups/models.py 65% <100%> (ø) ⬆️
geonode/base/models.py 74.21% <100%> (ø) ⬆️
...ode/maps/migrations/0027_remove_map_content_map.py 100% <100%> (ø)
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e1ca1d8...ec76b2d. Read the comment docs.

@capooti capooti changed the title Implements GNIP #3718 (Worldmap Client) Implements GNIP #3718 (Worldmap contrib application) May 30, 2018
of multi-disciplinary, multi-source and multi-format data,
organized spatially and temporally.</p>
"""
# these are optionals
Copy link
Member

Choose a reason for hiding this comment

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

Are there instructions and documentation on how to handle those optional settings?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, I have added a page in the documentation:
https://github.com/GeoNode/geonode/blob/2.7.x/docs/reference/worldmap.txt

I will try to improve it as soon as possible.

thanks!

@afabiani afabiani merged commit c9b5f84 into GeoNode:2.7.x Jun 5, 2018
afabiani pushed a commit to geosolutions-it/geonode that referenced this pull request Jun 6, 2018
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.

7 participants