WordPress.org

Make WordPress Themes

Opened 2 years ago

Closed 22 months ago

#27712 closed theme (live)

THEME: Magnus – 2.4

Reported by: hugobaeta Owned by: kevinhaig
Priority: previously reviewed Keywords: theme-magnus
Cc: hugo@…, karmatosed

Description

Magnus - 2.0

Life is Grand, Show it!

Theme URL - http://thefivethemes.com/themes/magnus
Author URL - http://thefivethemes.com

SVN - https://themes.svn.wordpress.org/magnus/2.0
ZIP - https://wordpress.org/themes/download/magnus.2.0.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=magnus/1.1.1&new_path=magnus/2.0

History:

Ticket Summary Status Resolution Owner
#8946 THEME: Magnus - 1.0 closed closed-newer-version-uploaded prosefectionist
#9154 THEME: Magnus - 1.1 closed closed-newer-version-uploaded
#9265 THEME: Magnus - 1.1.1 closed not-approved tskk
#27712 THEME: Magnus – 2.4 closed live kevinhaig

(this ticket)


https://themes.svn.wordpress.org/magnus/2.0/screenshot.png

Change History (32)

#1 @themetracbot
2 years ago

  • Owner set to asiathemes
  • Status changed from new to reviewing

#2 @karmatosed
2 years ago

  • Owner asiathemes deleted

I am sorry this review is taking so long. Sometimes people are unable to carry on the review, this may have happened this time. As a result, I am going to add this to the new queue again as a priority. This may mean it still takes time, but will prevent this ticket being held for so long by a reviewer that isn't able to carry on. Thanks for your patience.

If you are the reviewer and able to do this review, please carry on and request you get added back in Slack #themereview.

#3 @themetracbot
2 years ago

  • Owner set to faisalawan.isee

#4 @faisalawan.isee
2 years ago

  • Status changed from reviewing to approved

#5 @karmatosed
2 years ago

@faisalawan.isee can you please give more information than just marking it for approval. There is a great note on workflow here: https://make.wordpress.org/themes/handbook/review/review-workflow/.

If you are unable to do a full review, please respond and we'll make sure this theme gets a priority one. We need to know it's had a review or not. I'm not totally convinced in the short time it has.

#6 @karmatosed
2 years ago

  • Cc karmatosed added

#7 @hugobaeta
23 months ago

@karmatosed seems like @faisalawan.isee isn't keeping track. Is there anything I can do to help? (no hurries, just trying to understand what happens next)

#8 @karmatosed
23 months ago

@hugobaeta Hmm sorry about that. Let me try and get this someone to go over and check soon.

This ticket was mentioned in Slack in #themereview by karmatosed. View the logs.


23 months ago

#10 @kevinhaig
23 months ago

  • Status changed from approved to reopened

#11 @kevinhaig
23 months ago

  • Owner changed from faisalawan.isee to kevinhaig
  • Status changed from reopened to reviewing

#12 @kevinhaig
23 months ago

Magnus 2.0
Theme Review
======================
I am @kevinhaig and I picked up the review of your theme.

The review process follows procedures found in the Theme Handbook-> https://make.wordpress.org/themes/handbook/. If you do not understand a requirement or anything else in the review, or if you do not agree with anything, please comment in the ticket. I can then help you, or if I am not sure of something I will certainly seek a second opinion from an admin.

Nice Theme….just a few things to do.

Outcome

  • Theme left as reviewing for 7 days, and it may be closed after that if there is no response. Note that if you need more time, please make a comment in the ticket.

Required

Code

  • Please remove rtl.css as it is not being used.
  • Provide a unique prefix for everything the Theme defines in the public namespace, including options, functions, global variables, constants, post meta, etc.

Documentation

Licensing

  • Declare copyright and license explicitly. Use the license and license uri header slugs to style.css.
    • You have not properly declared the copyright. As a minimum, either in style.css or in the readme.txt file you need to have :
         theme name WordPress Theme, Copyright (C) 2015, theme author
         theme name is distributed under the terms of the GNU GPL
      
  • Declare licenses of any resources included such as fonts or images, including screenshot images
    • where possible list their copyright declaration and provide a link to the resource site, or image download link
    • Please provide a license and download link for the screenshot image so GPL compatibility can be confirmed.
    • Please delare jquery.fullPage.js, a readme.txt is a good place to do this.
  • ref: https://make.wordpress.org/themes/2014/07/08/proper-copyrightlicense-attribution-for-themes/

#13 @hugobaeta
23 months ago

Hey @kevinhaig! Yay, thank you for the proper review! I'm gonna work on these as soon as I have time for it. Please hold the ticket open for me. Thank you!

#14 @kevinhaig
22 months ago

It's been 7 days, still planning an update?

#15 @hugobaeta
22 months ago

Hey @kevinhaig - yes, just have been swamped with work and haven't been able. I'll do this during next weekend. Please keep the ticket open, thank you!

#16 @kevinhaig
22 months ago

It's not normal to allow this long of a wait, but given your previous wait, I will allow it :)

Admins, please do not close this ticket.

#17 @themetracbot
22 months ago

  • Summary changed from THEME: Magnus – 2.0 to THEME: Magnus – 2.1

Magnus - 2.1

Life is Grand, Show it!

Theme URL - http://thefivethemes.com/themes/magnus
Author URL - http://thefivethemes.com

SVN - https://themes.svn.wordpress.org/magnus/2.1
ZIP - https://wordpress.org/themes/download/magnus.2.1.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=magnus/2.0&new_path=magnus/2.1

History:

Ticket Summary Status Resolution Owner
#8946 THEME: Magnus - 1.0 closed closed-newer-version-uploaded prosefectionist
#9154 THEME: Magnus - 1.1 closed closed-newer-version-uploaded
#9265 THEME: Magnus - 1.1.1 closed not-approved tskk
#27712 THEME: Magnus – 2.4 closed live kevinhaig

(this ticket)


https://themes.svn.wordpress.org/magnus/2.1/screenshot.png

#18 @hugobaeta
22 months ago

@kevinhaig thanks for giving me a little extra time. as promised, I uploaded a new version with the fixes you requested. Let me know what is the next step. Thank you!

#19 @kevinhaig
22 months ago

Sorry, but I am on holidays for a couple of weeks. If I have time I will do the review, otherwise it will have to wait until I get back.

#20 @themetracbot
22 months ago

  • Summary changed from THEME: Magnus – 2.1 to THEME: Magnus – 2.2

Magnus - 2.2

Life is Grand, Show it!

Theme URL - http://thefivethemes.com/themes/magnus
Author URL - http://thefivethemes.com

SVN - https://themes.svn.wordpress.org/magnus/2.2
ZIP - https://wordpress.org/themes/download/magnus.2.2.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=magnus/2.1&new_path=magnus/2.2

History:

Ticket Summary Status Resolution Owner
#8946 THEME: Magnus - 1.0 closed closed-newer-version-uploaded prosefectionist
#9154 THEME: Magnus - 1.1 closed closed-newer-version-uploaded
#9265 THEME: Magnus - 1.1.1 closed not-approved tskk
#27712 THEME: Magnus – 2.4 closed live kevinhaig

(this ticket)


https://themes.svn.wordpress.org/magnus/2.2/screenshot.png

#21 @hugobaeta
22 months ago

@kevinhaig I had to upload a new version. Hadn't noticed that when I prefixed all the things, I ended up creating some errors. Version 2.2 fixes that and includes a new theme screenshot (and reference to author in both style.css and readme.txt). also updated readme.txt to include a Changelog and Credits sections.

Proceed with the review whenever possible, please don't take time from your vacation unless you want to. I can wait the two weeks. :)

This ticket was mentioned in Slack in #themereview by karmatosed. View the logs.


22 months ago

#23 @themetracbot
22 months ago

  • Summary changed from THEME: Magnus – 2.2 to THEME: Magnus – 2.3

Magnus - 2.3

Life is Grand, Show it!

Theme URL - http://thefivethemes.com/themes/magnus
Author URL - http://thefivethemes.com

SVN - https://themes.svn.wordpress.org/magnus/2.3
ZIP - https://wordpress.org/themes/download/magnus.2.3.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=magnus/2.2&new_path=magnus/2.3

History:

Ticket Summary Status Resolution Owner
#8946 THEME: Magnus - 1.0 closed closed-newer-version-uploaded prosefectionist
#9154 THEME: Magnus - 1.1 closed closed-newer-version-uploaded
#9265 THEME: Magnus - 1.1.1 closed not-approved tskk
#27712 THEME: Magnus – 2.4 closed live kevinhaig

(this ticket)


https://themes.svn.wordpress.org/magnus/2.3/screenshot.png

#24 @hugobaeta
22 months ago

Sorry about uploading a new version, found another small bug (again, caused by the "prefix all the things" issue).

#25 follow-up: @jcastaneda
22 months ago

caused by the "prefix all the things" issue

It happens. I had a feeling that would anyway. Core did a strange way of naming those I feel. :D

I'll try to get to this within the next 24 hours. From a quick scan it does look to be fairly good, would rather make for certain things do look good. :)

Though not sure if you meant to include the gruntfile this time around. I'm okay with that since it can make other developers easily contribute back to the source.

#26 in reply to: ↑ 25 @hugobaeta
22 months ago

Replying to jcastaneda:

Though not sure if you meant to include the gruntfile this time around. I'm okay with that since it can make other developers easily contribute back to the source.

Oh, right. Yeah, it's not meant to be there. Let's do this, let the review proceed, and when y'all tell me if there are any more change needed or not, I'll update it to remove the gruntfile, how about that?

FYI: this theme (as well as upcoming ones I'm working on) are open to collaboration (hence the gruntfile there). Development is being done on GitHub and is open for issues and pull requests ;) - https://github.com/TheFiveThemes/magnus

#27 @jcastaneda
22 months ago

open to collaboration

I figured as much and great to see that. :)


Just a few things that do need a little TLC ( not the group )

header.php

  • escape the description: L32 [required]
  • L64? [question, see below]
    • two post thumbnails?
    • can use get_the_ID() instead of $post->ID

functions.php

  • no editor-style.css exists but adding to visual editor [remove]
  • enqueuing genericons but no files exist [fix]

content.php

  • from what I can gather it is not being used or called by any other file [can be removed]

readme.txt

  • misspelled photoblogging under Tags [fix]

Notes

  • In the header.php you use: echo get_the_post_thumbnail( $post->ID, 'magnus-large' ); This is essentially the_post_thumbnail() so what can do is simply use the_post_thumbnail( 'magnus-large' ) that way you're not calling $post->ID, in which you can use get_the_ID() so you don't worry about the $post global. You could also remove that call for the_post_thumbnail seeing how it would print out two images when in a singular view. Not entirely sure you want to go that route but something to think about.
  • Commented code? I noticed some JavaScript that was being commented and also the social menu registration being commented out. If you are not going to use that be sure to remove it
  • Description? A little lacking I feel. If you were selling a car would you use that description? Don't get me wrong it's a great looking theme but it's not going anywhere when all you say is: Life is grand. Show it! What is the theme capable of? What plugins does it support? Can it wash my car? How can it show life is grand?

#28 @hugobaeta
22 months ago

I'm sorry I haven't had time to update the theme from this last review. I will check these tomorrow, please leave ticket open if possible! Thank you!!

#29 @themetracbot
22 months ago

  • Summary changed from THEME: Magnus – 2.3 to THEME: Magnus – 2.4

Magnus - 2.4

Life is Grand, Show it! Magnus is a photoblogging WordPress theme named after the latin word for Grand. It was made to showcase photos in a big and bold way, making the featured image on each post span the full width of the browser window, paired up with beautiful typography and a carefully planned layout.

Theme URL - http://thefivethemes.com/themes/magnus
Author URL - http://thefivethemes.com

SVN - https://themes.svn.wordpress.org/magnus/2.4
ZIP - https://wordpress.org/themes/download/magnus.2.4.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=magnus/2.3&new_path=magnus/2.4

History:

Ticket Summary Status Resolution Owner
#8946 THEME: Magnus - 1.0 closed closed-newer-version-uploaded prosefectionist
#9154 THEME: Magnus - 1.1 closed closed-newer-version-uploaded
#9265 THEME: Magnus - 1.1.1 closed not-approved tskk
#27712 THEME: Magnus – 2.4 closed live kevinhaig

(this ticket)


https://themes.svn.wordpress.org/magnus/2.4/screenshot.png

#30 @hugobaeta
22 months ago

@jcastaneda updated the theme with your suggestions, with the exceptions of removing the content.php (I'm under the impression it would act as a fallback if there's an issue, right?), and removing the commented lines (I need to work on implementing those but for now I don't have time, is that ok?).

Let me know if it looks ok! Thanks for the patience!

#31 @jcastaneda
22 months ago

I'm under the impression it would act as a fallback if there's an issue, right?

It would be a fallback for occasions when you are using get_template_part( 'content', 'something-else' )

The calls I see are:

get_template_part( 'content-home', get_post_format() ); // which uses content-home-{$format}.php 
get_template_part( 'content-archive', get_post_format() ); // which uses content-archive-{$format}.php
get_template_part( 'content', 'page' ); // uses content-page.php
get_template_part( 'content', 'search' ); // uses content-search.php
get_template_part( 'content', 'single' ); // uses content-single.php
get_template_part( 'content', 'none' ); // uses content-none.php

So content.php will never really be used.

Things look good, I see no other issues, and if Kevin agrees can be good to go.

#32 @kevinhaig
22 months ago

  • Resolution set to live
  • Status changed from reviewing to closed

Sure no problem.

Note: See TracTickets for help on using tickets.