This document specifies the basic principles behind the Magic Modules and its "golden rules" that shall not be infringed without a very strong (and documented) reason.
In this doc the terms:
- must: means something that have to be followed at all times
- should: means something that have to be followed as much as possible
Please refer to the top-level README and Philosophy before reading this guide. All code-reviews of Magic Modules (regardless of changes to core, providers, or products) must follow the guidelines outlined here.
- Development Principles
- Folder Locations & Usage
- Code Style
- Templates
- Code Reviews
- Exceptions
- Ruby Best Practices & Style Guide
- All changes must be tested for:
- Unit tests
- Code style compliance
- Changes to Provider Independent code must be tested against all providers
- Changes to Product Independent code must be tested against all products
- Changes to core Magic Modules features (which are both Product and Provider independent) must be tested against all products and all providers.
- Code coverage must always stay >80% and should be >90%
Resource URL and other self link constructs must not be exposed to customers. Use resource reference properties instead.
For the cases where a URL is unavoidable, provide a function that constructs the
URL based on the required properties of the resource being referenced. For
example to build an image family source disk a function similar to this is to be
provided: gcompute_address_self_link(name, region, project).
Definitions:
- Folders:
- Folders suffixed with
...means folder and its children. - Folders without
...represents the folder only.
- Folders suffixed with
- Provider Independent: Provider specific code, files or specializations must not be placed in this area
- Product Independent: Product specific code, files or specializations must not be placed in this area
api: Holds all definitions of objects used for defining products, serialization and deserialization- product independent
- provider independent
products/<product>/...: Holds all definitions, files, helpers, etc required to build the product, for all providersapi.yaml- provider independent
examples/<provider>helpers/files/
provider: All core provider features- product independent
- provider independent
provider/<provider>/...: Holds all provider specific code- product independent
templates: Holds all global templates- product independent
- provider independent
templates/<provider>/...: Holds all templates for the provider- product independent
Corollary: To completely remove (or add) a product to Magic Modules, removing (or adding) the
products/<product>/...folder is all it takes.
- All Ruby code must strictly abide by Rubocop standards.
Specializations to
.rubocop.ymlshould be avoided at all costs. - All RSpec code must strictly abide by rspec standards
- Line Length must be:
- Ruby: 80 chars
- YAML:
- Magic Modules YAML: 80 chars
- Product specific YAML: 80 chars
- Provider specific YAML: up to provider standards
- Markdown: 80 chars
Template embedded code should be as simple as possible. Avoid complicated logic in the templates because:
- It is harder to read and maintain
- It cannot be easily unit tested
It is best to only use simple iterators and provider specific functions. If you
have a complicated logic (e.g. 'build a map between URL and properties') it is
best to put this functionality in the provider/<provider>.rb class. All
methods in the provider class are directly available to the template (e.g. the
indent() function).
If the function is useful for all providers then consider adding it to the global (core) provider namespace.
All code changes must be reviewed by at least 1 other developer, following these guidelines:
- Product changes: change must be reviewed by a developer responsible (or very familiar) with the product being changed
- Provider changes: changes must be reviewed by a developer responsible (or very familiar) with the provider being changed
- Core changes: changes must be reviewed by a member of Magic Module's core team
A developer may be a member of 2+ groups and as long that every changed group has a developer that satisfies above it is okay to proceed.
Any permanent exceptions to these rules must be thoroughly documented in the code. If a longer discussion is required and becomes beyond the code where it lives an issue must be created and referenced for context.
For any temporary exceptions a tracking issue must be filed and added as a "TODO" in the code for future fixing.
Exceptions have to have the LGTM (approval) core team member, or from another core team member if the exception is introduced by a core team member.
To make the code easier to be maintained Magic Modules strives to have a consistent style guide and general rules outside regular Ruby style guides. These rules must be followed.
Ruby code must observe the rules in the Ruby style guide. They help build better Ruby code.
We want to keep the inline Ruby following general Ruby guidelines (<80 chars) and at the same time not dictating how the target files should look like. For example Go does not have a maximum line limit and it is up to the writer to break at will (usually following some project wide guidelines set by themselves). So to keep the best of both worlds:
- All inline Ruby code must fit <80 characters
- Generated code (and its non-inline Ruby text) is up to the writer's discretion
Good
<%= lines(indent('hello', 10)) -%>
Bad
<%=
lines(indent('hello', 10))
-%>
Use lines() or lines_before() functions instead.
This both helps avoid tracking source of newlines and accidentally add more/less
spaces than needed
Good
<%= lines("this has 3 new lines afterwards" + some_function(), 3) -%>
Bad
<%= "this has 3 newlines afterwards" + some_function() + "\n\n\n"" -%>
In the example above you don't know how many blank lines some_function() would
return.
Do not .join("\n"). Use lines() instead.
To keep spotting where \n are added is cumbersome and error prone. Use
lines function to ensure that your array is properly formatted.
If you need to break line use lines() function instead.
Good
<%= lines(my_var) -%>
Bad
<%= my_var %>
Although it seems easier to not use the lines() function, it is
common to have formatting bugs due to mixing -%> and %>.
By rooting <% (and -%> if multiline) to column 1 it will avoid introducing
spurious spaces in the final output.
Good
|----------
<%=
lines(["something goes here", "and here"])
-%>
|----------
Produces:
|----------
something goes here
and here
|----------
Bad
|----------
<%=
lines(["something goes here", "and here"])
-%>
|----------
Produces:
|----------
something goes here
and here
|----------
If you intend to have spaces in the beginning it is easier to clearly show that,
e.g. with indent(), so someone does not try to "fix" your code.
Good
<%
my_code
my_code
-%>
Bad
<%
my_code
my_code -%>
It is common for a template to have various lines between start and end of a block and tracking them can become hard in complex templates. Adding something to remind you where they belong improves readability
<% objects.each do |obj| -%>
...
<% if obj.readonly -%>
...
... many lines later
...
<% elsif !obj.readonly && obj.broken -%>
...
... many lines later
...
<% end # if obj.readonly -%>
...
... many lines later
...
<% end # objects.each -%>
It will most of time make it easier to see that a block depends on another. In
the example below it is easier to note the if is inside the each and there are
2 cascaded ifs as well
Good
<% objects.each do |obj| -%>
...
<% if obj.readonly -%>
...
<% if obj.input -%>
...
... many lines later
...
<% end # if obj.input -%>
<% end # if obj.readonly -%>
...
... many lines later
...
<% end # objects.each -%>
Bad
<% objects.each do |obj| -%>
...
<% if obj.readonly -%>
...
<% if obj.input -%>
...
... many lines later
...
<% end # if obj.input -%>
<% end # if obj.readonly -%>
...
... many lines later
...
<% end # objects.each -%>
Use format() for input dependent (style violating) output
Rely on format() function for generated code that depends on
variables or other input specific data that may affect the generated output.
Good
<%=
format([
["this will fit in 1 line. user = #{user.full_name}"],
[
"this will fit in 2 lines.",
user.full_name
],
[
"ugh, not even in 2 lines :(. let's make it 3 then",
user.first_name,
user.last_name
]
], 10, 40)
-%>
Bad
<%
# Try to calculate the effective size of strings
one_liner = "this will fit in 1 line. user = #{user.full_name}"
-%>
<% if one_liner.length + 10 < 40 -%>
<%= lines(one_liner) -%>
<% else # one_liner.length did not fit -%>
<%
# Try to calculate if fits in 2 lines
...
...
...
-%>
...
...
...
<% end # one_liner.length -%>
Please refer to format() documentation for usage examples.
Prefer nested indent() and indent_list() over calculating its relative displacements
Good
<%=
indent([
'first level',
indent([
'second level',
'also on second level'
], 2),
'also on first level'
], 2)
-%>
Bad
<%=
indent('first level', 2)
indent('second level', 4)
indent('also on second level', 4)
indent('also on first level', 2)
-%>
It makes it easier to read the code if you split the steps into functional phases.
Good
<%=
# List all readonly properties that are not nested objects alphabetically
lines(object.all_properties.select(&:readonly)
.reject { |p| p.is_a?(Api::Type::NestedObject) }
.sort
.map { |p| "- #{p.out_name}" })
-%>
Bad
<%
# List all readonly properties that are not nested objects alphabetically
my_properties = []
object.all_proerties.each do |p|
if p.readonly && !p.is_a?(Api::Type::NestedObject)
my_properties << p
end # p.readonly
end # object...each
%>
<% my_properties.sort.each do |p| -%>
- <%= p.out_name -%>
<% end # my_properties.each -%>
Ruby's Enumerable interface contains a list of methods that can be used in such cases.