Web Site Project Page: Difference between revisions

From GCD
Jump to navigation Jump to search
(Add template style rules that have developed over time.)
m (new technical issues)
 
(28 intermediate revisions by 4 users not shown)
Line 1: Line 1:
== GCD Web Site Project Master Page ==
== GCD Web Site Project Master Page ==


The GCD is currently implementing a new web site to reflect the growth of the project and changes in available technology. This page summarizes the project status and provides links to information on various aspects of the work.
The GCD has recently moved to an entirely new server and code base! This page documents the ongoing work to implement more features and improve the new system, the initial deployment of which was somewhat rushed by the abrupt demise of our prior technical arrangement.
 
Our top priorities are:
* Improving our ability to credit and document contributions
* Improving the structure of our data for better display and search
* Improving our user interface (*we need a web designer for this*)
* Improving our search capabilities


We welcome assistance from programmers and non-programmers alike (yes, we have tasks for non-technical people!).
We welcome assistance from programmers and non-programmers alike (yes, we have tasks for non-technical people!).
== Releases and Status ==
Projected releases are named after comic books, and are planned in detail on their own pages.  Release numbering is tentative as intermediate releases may be added or removed as plans and features are revised.
{| border="1"
|+ '''GCD Site Releases by Code Name'''
|-
! Name !! # !! Purpose !! Next Milestone !! Comments
|-
| '''New Fun''' * [[Release New Fun|requirements]] * [[New Fun Design|design]] || 1.0 || First functional, production-ready release. Will contain the minimum functionality needed to migrate from the old site.|| Technical Design || Named for the first U.S. comic book with all-new content.
|-
| [[Release More Fun|More Fun]] || 1.1 || Minor release for small items that are not critical for inclusion in New Fun, or that require us to already have control of the database. || || As the comic "New Fun" became "More Fun", this release is just a small next step.
|-
| [[Release The Dandy|The Dandy]] || TBD || First release to include major steps towards the new schema.  There may be one or more small releases between More Fun and The Dandy || || Named for the very long-running British Weekly comic.  Also, this release should be quite [http://en.wiktionary.org/wiki/dandy dandy] :-)
|}


== Development Environment ==
== Development Environment ==


* Mailing List: [http://groups.google.com/group/gcd-tech gcd-tech at Google Groups]
* Mailing List: [http://groups.google.com/group/gcd-tech gcd-tech at Google Groups]
* Bug Tracking: [http://dev.comics.org/bugs Bugzilla (GCD Tech version)]
* Source Code: [https://github.com/GrandComicsDatabase/gcd-django gcd-django at github]
* Source Code: [http://sourceforge.net/projects/grandcomic-book Subversion hosted by SourceForge]
* Bug Tracking: GitHub Issues (new work), with some old reports still in [http://dev.comics.org/bugs Bugzilla (GCD Tech version)]
* Code Reviews: [http://dev.comics.org/reviews/ Review Board (GCD Tech installation)] (please read the [http://www.review-board.org/docs/manual/dev/users/ Users' Guide])
* Code Reviews: [http://reviews.comics.org/ Review Board (GCD Tech installation)] (please read the [http://www.review-board.org/docs/manual/dev/users/ Users' Guide])
 
Figuring out what all that stuff in GitHub actually is, and which parts we're really using:
* [[Developer's Guide to the Code]]


How to set up a development environment:
How to set up a development environment:
* [http://grandcomic-book.svn.sourceforge.net/viewvc/grandcomic-book/pydjango/docs/Getting_Started_on_MacOS.txt MacOS ]
 
* [http://grandcomic-book.svn.sourceforge.net/viewvc/grandcomic-book/pydjango/docs/Getting_Started_on_Windows.txt Windows] or [[Getting Started on Windows]]
Please use the [https://github.com/GrandComicsDatabase/gcd-django-vagrant-install GCD Vagrant-based development environment]


Please join the mailing list and email us if you would like to help out!
Please join the mailing list and email us if you would like to help out!
Line 39: Line 32:
* [http://docs.djangoproject.com/ Django]
* [http://docs.djangoproject.com/ Django]
* [http://dev.mysql.com/doc/refman/5.0/en/ MySQL]
* [http://dev.mysql.com/doc/refman/5.0/en/ MySQL]
* [http://selenium.openqa.org/ Selenium]
* [http://www.elasticsearch.org/guide/ Elasticsearch]
* [http://django-haystack.readthedocs.org/en/latest/ Haystack]


== Standards and Best Practices ==
== Bug/Issue Tracking and Workflow Management ==


Code submitted to the project should follow these standards.  [http://www.review-board.org/ Review Board] has been set up for code reviews, and all changes '''must''' go through code review and be approved '''before''' being checked in.  Some of these guidelines are covered in places such as the Django documentation, but the ones restated below are items we feel are important enough to be called out here.
New issues should be filed and tracked in GitHub.


=== Using Review Board ===
Issues are available for anyone to work on. If someone is assigned to an issue contact that person before on it.


Most of what you need can be found in the Review Board [http://www.review-board.org/docs/manual/dev/users/ User's Guide].  Don't forget to download the <tt>post-review</tt> script as explained in the [http://www.review-board.org/docs/manual/dev/users/tools/post-review/ post-review] documentation.  You should already be running Python 2.4 or higher.  You may need an additional library or two- the documentation should explain where to get it (probably through [http://www.python.org python.org]).
The following issues were voted on in 2018 and have not yet been implemented:
*Company Credits - https://www.comics.org/voting/ballot/335/
* Story Credits - https://www.comics.org/voting/ballot/337/


==== To Submit a Review Request ====
== Standards and Best Practices ==
 
When you are ready to submit code for review, if you have two files, <tt>apps/gcd/views/foo.py</tt> and <tt>templates/gcd/bar.html</tt> then you would run the command:
 
<code>
    post-review --server=http://dev.comics.org/reviews/ apps/gcd/views/foo.py templates/gcd/bar.html
</code>
 
If those two files are the '''only''' files in your tree that have changed, you don't have to specify them individually- the script should find them automatically.  But if you have multiple files edited, added or deleted, and don't want to submit all of them, you must tell the script which ones to look at.  Note also that the script should remember your --server setting, but if you're having trouble with it, try passing that option.
 
Once you've done this, login to review board on maelstrom and you should see that you have a review marked as an "Outgoing Review" in your dashboard (there will be a '1' next to those words on the left side).  You must fill in some extra information and publish the review before others can see it.
 
Currently, the review board developers are working to fix a number of usability problems before their upcoming 1.0 release.  Hopefully things will be more obvious then.  For now, the UI is sometimes a bit counter-intuitive.  Here's what you have to do in the page where you edit the review:
 
* Edit fields by clicking on the pencil icons next to them.  When you are done with the fields, hit enter [return] to save the field.
** Fields that are multi-line text boxes will have an OK button.
** Summary, Description, and Groups are required.  You should not use the People field, but should instead always set Groups to "gcd" (without the quotes).
** You should put the bug number (from the [http://dev.comics.org/bugs GCD Tech Bugzilla]) in the Bugs field whenever there is a relevant bug.  Even if the change does not completely fix the bug.
** Filling out Testing Done is strongly encouraged.
** When proposing a significant change to the visual layout of the site, a screenshot is often useful.  There is an "Attach Screenshot" link at the bottom right-hand corner of the panel for this purpose.
 
You can also click View Diff in the upper right-hand corner to make sure the changes being shown are the ones you intended.
 
Once you've filled out the fields, click Publish.  You may need to click more than once if the system thought you were still editing one of the other fields (but wait and see if the first click worked for a bit- it should send email to the gcd-tech mailing list).
 
==== To Review Someone Else's Code ====
 
Click on the link in the email.  Click View Diffs in the upper right-hand corner to look at the changes.  Click on any line number to make notes about the code, and write your comments in the Comments tab, clicking Save Comment when you are done with the comments for that line.  You will see your saved comments appear in the Reivew tab.
 
You may view existing comments (noted by a speech bubble next to a line number) by clicking that line number and looking in the Discussion tab.
 
When you have finished adding comments and responding to the discussion, click "Review" (which is a small link at the bottom right of the top panel of the review, above the diffs, or click any line number and go to the "Review" tab.  If you want to send your comments without approving the code (because you are asking for changes or explanations), click Publish.  If you approve of the review request, click Ship It.
 
(Note:  "Ship it!" has long been the traditional approval response at VMware, from which this tool originates.  It is amusing to watch the Review Board authors defend this phrasing on mailing lists and bug reports instead of just changing it to "Approve" or something else more obvious to non-VMware people.  It's important to note that the Ship It button does '''not''' check the code in.  The developer who submitted the code must do that).
 
This review process repeats each time the author modifies the change in response to comments until the reviewer(s) are satisfied and approve the review.  Unresolvable disputes may be appealed to the Head Programmer, or if the Head Programmer is one of the parties, to the Technical Coordinator.
 
==== To Make Changes to a Review Request ====
 
If you are asked to make changes to a review you submitted (and agree to make the changes), then after you have finished your changes, re-run <tt>post-review</tt>, but with the <tt>-r</tt> argument giving the existing review number for your request.  This will cause the tool to update the diff of your review instead of creating a new one.  For instance, following the original example (and assuming no new files were added to the diff, or old ones deleted from it), and assuming the review is number 42, the command would be:
 
<code>
    post-review --server=http://dev.comics.org/reviews/ -r 42 apps/gcd/views/foo.py templates/gcd/bar.html
</code>
 
(Note that the script should remember your server, so you probably won't need --server all the time unless you're using review-board somewhere else as well)
 
Once you've done that, you once again go to the review, make any additional comments or respond to discussions in the same way you would if you were reviewing the code yourself, and then click Publish again.  You may also make comments and change fields without submitting a new diff.  Be sure to hit return if you are changing any of the fields to save your edits before you click Publish.
 
==== Committing the Code ====


Once a reviewer has approved the change, you should check in using <tt>svn</tt> the same way you always haveAfter that, you should go back to Review Board, go to the review screen, and click the "Set Submitted" link in the bottom right-hand corner of the main review panel (do ''not'' click View Diffs first, as the Set Submitted link does not appear on that page)This will remove the review request from the active list of reviews in everyone's dashboard.
Code submitted to the project should follow these standardsAll changes go through code review and must be approved '''before''' being checked inSome of these guidelines are covered in places such as the Django documentation, but the ones restated below are items we feel are important enough to be called out here.


==== Abandoning a Change ====
=== Using Pull Requests ===


If, based on feedback or your own reconsiderations, you decide not to check in a change, then instead of checking in and clicking "Set Submitted", go to the same location and click "Discard Review". This also removes the request from the list of active reviews.
See [https://help.github.com/articles/about-pull-requests/ pull requests on github]


=== Standards For Python Code ===
=== Standards For Python Code ===
; Write Python 2.4 compatible code.
; Write Python 2.7 compatible code.
: This is in part because of compatibility problems with a dependent package on Mac OS 10.4But also because hosting environments might not all be up to 2.5 yet (although that is less of a concern as time goes on). Your code should be compatible with Python 2.4-2.6. 
: This is our current production environmentPython 3 is not yet supported.


; Follow the [http://www.python.org/dev/peps/pep-0008/ Python Style Guide]. Read the whole thing, as it is full of useful advice.
; Follow the [http://www.python.org/dev/peps/pep-0008/ Python Style Guide]. Read the whole thing, as it is full of useful advice.
Line 126: Line 73:


Breaking with an "obvious" indentations scheme of lining up operands vertically:
Breaking with an "obvious" indentations scheme of lining up operands vertically:
<code>
<pre>
     foo = (bar.some_numeric_thingy +
     foo = (bar.some_numeric_thingy +
           some_other_number)
           some_other_number)
</code>
</pre>


Breaking when there is no obvious indentations scheme (so two spaces are used):
Breaking when there is no obvious indentations scheme (so two spaces are used):
<code>
<pre>
     SomeClass.some_list_of_things[indexy_bit].big_long_method_call(
     SomeClass.some_list_of_things[indexy_bit].big_long_method_call(
       some_long_variable_name)
       some_long_variable_name)
</code>
</pre>


Note that it's often better to just break things up into multiple statements by using a few local varaibles in these situations:
Note that it's often better to just break things up into multiple statements by using a few local varaibles in these situations:
<code>
<pre>
     thingy = SomeClass.some_list_of_things[indexy_bit]
     thingy = SomeClass.some_list_of_things[indexy_bit]
     thingy.big_long_method_call(some_long_variable_name)
     thingy.big_long_method_call(some_long_variable_name)
</code>
</pre>


; Do not use global variables.  Ever.
; Do not use global variables.  Ever.
Line 147: Line 94:


; Make everything portable, and especially use <code>os.path</code> when working with files and directories.
; Make everything portable, and especially use <code>os.path</code> when working with files and directories.
: The project currently has only three active developers and a shared test are, but already needs to run on MacOS, Windows and Linux.  We also do not yet know what our hosting environment will be.  So don't do anything that will limit us.
: The project currently has only three active developers and a shared test area, but already needs to run on MacOS, Windows and Linux.  We also do not yet know what our hosting environment will be.  So don't do anything that will limit us.


; When capturing from regular expressions, use named capture groups, not positional groups.
; When capturing from regular expressions, use named capture groups, not positional groups.
: This make regular expressions more robust if the string ever changes and capture groups get reorderd.  Additionally, while the regexp itself is a bit more cluttered, the intention of the capture groups is much more clear.  Examples:
: This make regular expressions more robust if the string ever changes and capture groups get reordered.  Additionally, while the regexp itself is a bit more cluttered, the intention of the capture groups is much more clear.  Examples:


Bad:
Bad:
<code>
<pre>
     m = re.search(r'foo:\s*(\w*), (\w*)', target)
     m = re.search(r'foo:\s*(\w*), (\w*)', target)
     foo = m.groups()[1]
     foo = m.groups()[1]
     bar = m.groups()[2]
     bar = m.groups()[2]
</code>
</pre>


Good:
Good:
<code>
<pre>
     m = re.search(r'foo:\s*(?P<foo>\w*), (?P<bar>\w*)', target)
     m = re.search(r'foo:\s*(?P<foo>\w*), (?P<bar>\w*)', target)
     foo = m.group('foo')
     foo = m.group('foo')
     bar = m.group('bar')
     bar = m.group('bar')
</code>
</pre>
 
=== For Perl Code ===
; Where possible, be compatible with the Python standards listed above.
: Exceptions are obvious language community differences such as packages being CamelCase in Perl while they are lower_case in Python.  Use Perl-style package names for Perl packages.  Other exceptions should be documented here as they are noted.


=== For Django Models ===
=== For Django Models ===
Line 170: Line 121:
: Models are pure data representation.  They should present what is in the database, and sometimes expose methods that make that data easier to work with.  But they should essentially be unaware of the higher layers of the application.  Note that model fields that contain URLs because the database contains URLs are OK.  These are generally external links anyway (like the URL field for publishers).
: Models are pure data representation.  They should present what is in the database, and sometimes expose methods that make that data easier to work with.  But they should essentially be unaware of the higher layers of the application.  Note that model fields that contain URLs because the database contains URLs are OK.  These are generally external links anyway (like the URL field for publishers).


:There is one notable exception to this rule, which is the get_absolute_url() method which is a Django convention for models.  The Head Programmer does not like this method, because it ties the model representation to a single URL when we may wish to have several different URLS (for instance, one for display purposes, and one for REST API purposes).  However, given that parts of Django expect this convention, it may be implemented and the basic display URL for the model should be used.
:There is one notable exception to this rule, which is the get_absolute_url() method which is a Django convention for models.  The Head Programmer does not like this method, because it ties the model representation to a single URL when we may wish to have several different URLs (for instance, one for display purposes, and one for REST API purposes).  However, given that parts of Django expect this convention, it may be implemented and the basic display URL for the model should be used.


=== For Django Views ===
=== For Django Views ===
Line 210: Line 161:


We have not yet determined how and to what extent we want to use JavaScript.  Please discuss any ideas with the Head Programmer first.  Input on this topic is welcome.
We have not yet determined how and to what extent we want to use JavaScript.  Please discuss any ideas with the Head Programmer first.  Input on this topic is welcome.
== Historical Note ==
Some interesting historical info that used to be on this page but became outdated enough to be confusing has been moved to [[Tech Archive]].
[[Category: GCD Technical]]

Latest revision as of 01:15, 3 December 2018

GCD Web Site Project Master Page

The GCD has recently moved to an entirely new server and code base! This page documents the ongoing work to implement more features and improve the new system, the initial deployment of which was somewhat rushed by the abrupt demise of our prior technical arrangement.

Our top priorities are:

  • Improving our ability to credit and document contributions
  • Improving the structure of our data for better display and search
  • Improving our user interface (*we need a web designer for this*)
  • Improving our search capabilities

We welcome assistance from programmers and non-programmers alike (yes, we have tasks for non-technical people!).

Development Environment

Figuring out what all that stuff in GitHub actually is, and which parts we're really using:

How to set up a development environment:

Please use the GCD Vagrant-based development environment

Please join the mailing list and email us if you would like to help out!

3rd-party Documentation

Bug/Issue Tracking and Workflow Management

New issues should be filed and tracked in GitHub.

Issues are available for anyone to work on. If someone is assigned to an issue contact that person before on it.

The following issues were voted on in 2018 and have not yet been implemented:

Standards and Best Practices

Code submitted to the project should follow these standards. All changes go through code review and must be approved before being checked in. Some of these guidelines are covered in places such as the Django documentation, but the ones restated below are items we feel are important enough to be called out here.

Using Pull Requests

See pull requests on github

Standards For Python Code

Write Python 2.7 compatible code.
This is our current production environment. Python 3 is not yet supported.
Follow the Python Style Guide. Read the whole thing, as it is full of useful advice.
This saves us from arguing over which of our personal styles should be used. Code that does not meet these guidelines will not be allowed through code review. If you find existing code that does not comply, please consider cleaning it up. Do not consider it an excuse to write more non-compliant code.
Read Python Idioms and Anti-Idioms
And do what it says :-)
Read the Docstring Guide
And do what it says :-)
Always place one space after each comma separating function arguments.
Unless of course there's a newline. For some reason this is not mentioned in the style guide, although all examples there follow it.
When breaking a statement across two lines, if there is no obvious way to indent it, indent two characters more than the start of the statement.
This makes it clear that the continued statement is not another indented block. When possible, line up continued statements by lining up arguments vertically, or the continued operand in an expression with the operand on the previous line. But when no such rule applies, use two extra spaces. Examples:

Breaking with an "obvious" indentations scheme of lining up operands vertically:

    foo = (bar.some_numeric_thingy +
           some_other_number)

Breaking when there is no obvious indentations scheme (so two spaces are used):

    SomeClass.some_list_of_things[indexy_bit].big_long_method_call(
      some_long_variable_name)

Note that it's often better to just break things up into multiple statements by using a few local varaibles in these situations:

    thingy = SomeClass.some_list_of_things[indexy_bit]
    thingy.big_long_method_call(some_long_variable_name)
Do not use global variables. Ever.
If for some reason you feel you must do this, you must get approval from the Head Programmer first. The current acting Head Programmer is unlikely to approve such a thing. If 3rd-party libraries or frameworks rely on global variables, you may of course use them as required.
Make everything portable, and especially use os.path when working with files and directories.
The project currently has only three active developers and a shared test area, but already needs to run on MacOS, Windows and Linux. We also do not yet know what our hosting environment will be. So don't do anything that will limit us.
When capturing from regular expressions, use named capture groups, not positional groups.
This make regular expressions more robust if the string ever changes and capture groups get reordered. Additionally, while the regexp itself is a bit more cluttered, the intention of the capture groups is much more clear. Examples:

Bad:

    m = re.search(r'foo:\s*(\w*), (\w*)', target)
    foo = m.groups()[1]
    bar = m.groups()[2]

Good:

    m = re.search(r'foo:\s*(?P<foo>\w*), (?P<bar>\w*)', target)
    foo = m.group('foo')
    bar = m.group('bar')

For Perl Code

Where possible, be compatible with the Python standards listed above.
Exceptions are obvious language community differences such as packages being CamelCase in Perl while they are lower_case in Python. Use Perl-style package names for Perl packages. Other exceptions should be documented here as they are noted.

For Django Models

Do not refer to Views, Templates or site URLs.
Models are pure data representation. They should present what is in the database, and sometimes expose methods that make that data easier to work with. But they should essentially be unaware of the higher layers of the application. Note that model fields that contain URLs because the database contains URLs are OK. These are generally external links anyway (like the URL field for publishers).
There is one notable exception to this rule, which is the get_absolute_url() method which is a Django convention for models. The Head Programmer does not like this method, because it ties the model representation to a single URL when we may wish to have several different URLs (for instance, one for display purposes, and one for REST API purposes). However, given that parts of Django expect this convention, it may be implemented and the basic display URL for the model should be used.

For Django Views

Use named groups in the URL conf for argument passing.
This makes view invocation more robust.
Use named URLs to disambiguate reverse mappings.
This supports better abstraction when code needs to look up URLs.

For Django Templates

We are still developing our template style
Suggestions appreciated. For now, stick with the following rules, and when you're working on a template that doesn't already conform, please try to leave it in better shape than you found it.
Template tags should have exactly one space inside of the enclosing punctuation
Example: {% if bar %} <span class="data">{{ foo }}</span> {% endif %}
Use two-space indentations
It's easy to get a lot of indentations in these things, so let's not get carried away.
Indent HTML and Django template tags independently
Trying to keep up with one indentation scheme results in weird misalignments when you just look at HTML or just look at template tags. So don't try. Indent all of your template tags consistently with each other. Indent all of your HTML tags consistently with each other. Don't worry if they don't make indentation sense together.
Please try to keep content within 80 columns, but do not go through confusing contortions in the process.
In other words, if you've got long URLs or if you need to string things together due to the vagaries of how Django's template engine performs substitutions, go ahead and let the line be as long as it needs to be. Sometimes, due to how Django templates treat space, this can be very long indeed.

For HTML

Use 2-character indentation for each nesting level.

For CSS

All CSS goes in external style sheets.
When modifying styles dynamically, it is better to modify the element's class(es) than set styles directly.
Classes and IDs are always lower_case_separated_by_underscores.
Colors are always associated with class names that define an elements role in the color scheme.
i.e. background, foreground, hilight, etc. Actual list TBD.

For JavaScript

We have not yet determined how and to what extent we want to use JavaScript. Please discuss any ideas with the Head Programmer first. Input on this topic is welcome.

Historical Note

Some interesting historical info that used to be on this page but became outdated enough to be confusing has been moved to Tech Archive.