widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

Review: Approve

some nits, otherwise lgtm. 

Diff comments:

> 
> === added directory 'media/wlmaps/minimaps'
> === modified file 'wlmaps/forms.py'
> --- wlmaps/forms.py	2016-02-15 14:06:09 +0000
> +++ wlmaps/forms.py	2016-08-21 13:20:05 +0000
> @@ -49,6 +55,17 @@
>              del cleaned_data['file']
>              return cleaned_data
>  
> +        try:
> +            # Move the minimap
> +            rename(MEDIA_ROOT + '/wlmaps/maps/' + file.name + '.png',

use os.path.join to avoid double slashes and stuff? os.path.join(MEDIA_ROOT, "wlmaps", "maps", file.name + '.png')

> +                   MEDIA_ROOT + '/wlmaps/minimaps/' + file.name + '.png' )
> +        except OSError:
> +            self._errors['file'] = self.error_class(
> +                ['The minimap could not be moved. Please inform an admin.'])
> +            del cleaned_data['file']
> +            
> +            return cleaned_data
> +
>          # Add information to the map
>          self.instance.name = mapinfo['name']
>          self.instance.author = mapinfo['author']
> @@ -60,7 +77,7 @@
>  
>          self.instance.world_name = mapinfo['world_name']
>          # mapinfo["minimap"] is an absolute path and cannot be used.
> -        self.instance.minimap = '/wlmaps/maps/' + file.name + '.png'
> +        self.instance.minimap = '/wlmaps/minimaps/' + file.name + '.png'

same here: fix the code to use os.path.join

>  
>          # the json file is no longer needed
>          default_storage.delete(name + '.json')
> 
> === modified file 'wlmaps/models.py'
> --- wlmaps/models.py	2016-08-11 18:13:59 +0000
> +++ wlmaps/models.py	2016-08-21 13:20:05 +0000
> @@ -23,8 +24,13 @@
>  
>      descr = models.TextField(verbose_name='Description')
>      hint = models.TextField(verbose_name='Hint')
> +    
> +    # Attribute 'upload_to=' has no effect here; saving of minimap is

is 'upload_to' a mandatory option or can we just leave it out?

> +    # processed in wlmaps.forms.UploadMapForm
>      minimap = models.ImageField(
>          verbose_name='Minimap', upload_to='wlmaps/minimaps')
> +    
> +    # The real filename of a map
>      file = models.FileField(verbose_name='Mapfile',
>                              upload_to='wlmaps/maps')
>  


-- 
https://code.launchpad.net/~widelands-dev/widelands-website/move_minimaps/+merge/303493
Your team Widelands Developers is subscribed to branch lp:widelands-website.