-
Notifications
You must be signed in to change notification settings - Fork 133
Add preview to the screenshot folder (for #29) #30
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
Conversation
codebrainz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the comments, this looks good. Nice work.
|
One more thought, why not generate this using a script so we don't have to remember to update it whenever a theme is added? If not using a script, then the ADDING-A-THEME file needs to be updated to mention adding the Markdown into this new README file. |
|
As to "generate this using a script", I've documented how at https://dev.to/suntong/generating-image-preview-easily-opj. However, for doing that in GH, that's something I didn't know and needs admin rights to the geany-themes repo too... Would you like me to include my template as well in next PR, @codebrainz? If so, would you like it under the |
|
I just meant a simple (ideally Python, since it's what's already used) script that scans the themes and prints the Markdown. The script would be run from the make file to regenerate the Markdown file whenever adding a new theme, like is already done with |
As per #30's new requirement
|
Verified that the changes are working at my end. |
codebrainz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me.
As mentioned in the comments, it would be nice to have another pull request that either generates this using a Python script (rather than using additional tools/templates), or to update ADDING-A-THEME reminding to update this file manually.
|
I don't know Python, but I think such screenshots generation, using Python or not, will only be a temporary solution, as a proper implementation at the server-side is the best option in the long run, since geany already has its web site, and that is where these things belong to. |
|
All Geany is provided by volunteer contributors, so "somebody" needs to contribute the website server side (and its Django so its still Python, see here). For the same reason anything that makes it easier to add themes here is useful. The more manual it is the more someone like @codebrainz has to prompt contributors to fix their pull requests every time they contribute a theme. |
|
Maybe it came out wrong, but before I even submitted the PR, I stressed that I have no objection of server-side implementation that you proposed @elextr, and mine will be only be a temporary solution before yours' done. Anyway, I don't know Python, so this is as far as I can help. |
|
@suntong agree, and I put somebody in quotes to indicate not you (or me :). The comment was so "somebody" who saw this and thought it was a good idea would know where to look for the website, its new, and being in that repo where its easy to contribute pull requests is new. Or if they didn't want to try the server side it was clear that a script here was still acceptable. |
|
Guys, in geany/www.geany.org#5 I already said I will work on this and now done in: geany/www.geany.org#8. |
|
Just to be clear, I see what this PR added (a screenshots/README.md to make browsing on Github nicer) as completely separate from the website as in geany/www.geany.org#5 and geany/www.geany.org#8. I added #31 to make this no more work than regenerating the indices and such (ie. running |
|
@eht16 didn't see geany/www.geany.org#5 because I'm not subscribed to that repo, but unless geany/www.geany.org#8 that also provides the ability to save the theme to a file directly from the page, users will still need to come here to get the themes, so as @codebrainz said its good to have both. |
|
I completely agree. The preview page on the website and the preview in the README here do not compete at all. It's perfectly fine to have both. |
No description provided.