Skip to content
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

Add dynamic shortlinks config with moban #94

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yukiisbored
Copy link
Member

Closes #4

devops.yml Outdated
url: "https://github.com/issues?utf8=%E2%9C%93&q=is%3Aopen+is%3Aissue+user%\
3Acoala+label%3Adifficulty%2Fmedium+no%3Aassignee"
- name: review
url: "https://github.com/pulls?q=is%3Aopen+user%3Acoala+label%3A%22process%2\
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (81 > 80 characters) (line-length)

Origin: YAMLLintBear, Section: all.yaml.

Copy link
Member

@jayvdb jayvdb Dec 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of a long link url, break it into parts like label: process/pending-review sort: created-asc

The problem then arises that there are many renders of the data, and they all need to do the same url conversion. So we need to have a shortlink-url.jinja2 template in coala-mobans which only renders the data into a url , and it is included in any higher level renderer. Not sure if this is supported by moban yet, but ... muck around a bit, ... and if it doesnt work ... I am very confident that the creator will add the feature once we've clearly defined it.

devops.yml Outdated
- name: greview
url: https://gitlab.com/groups/coala/merge_requests
- name: approved
url: "https://github.com/pulls?q=is%3Aopen+user%3Acoala+label%3A%22process%2\
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (81 > 80 characters) (line-length)

Origin: YAMLLintBear, Section: all.yaml.

.moban.yml Outdated
@@ -0,0 +1,7 @@
configuration:
template_dir:
- templates
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The recommended approach now is to put templates into a .moban.dt . That is why my PRs do. We need to choose the same approach , for consistency sake. c.f. https://github.com/coala/coala/pull/4897/files#diff-ce88a447f57dae1c1ad94204c8eb7dc5R40

.moban.yml Outdated
- templates
configuration: devops.yml
targets:
- "nginx/nginx.conf.d/coala.io.conf": "coala.io.conf"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename the templates foo.jinja2 , and add the bear to that filespec .

@@ -0,0 +1,125 @@
shortlinks:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file should go into coala-mobans , as shortlinks.yml

Then in .moban.yml , add configuration-dir: ../coala-mobans/ like the others.

An immediate use of it in another repo is validating links in the docs .

Copy link
Member Author

@yukiisbored yukiisbored Dec 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems coala/coala-mobans still doesn't exist and currently resides under your namespace.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am still waiting for reviews. Once it is approved, I will move it and update my PRs and then they can be merged.

devops.yml Outdated
url: "https://github.com/issues?utf8=%E2%9C%93&q=is%3Aopen+is%3Aissue+user%\
3Acoala+label%3Adifficulty%2Fmedium+no%3Aassignee"
- name: review
url: "https://github.com/pulls?q=is%3Aopen+user%3Acoala+label%3A%22process%2\
Copy link
Member

@jayvdb jayvdb Dec 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of a long link url, break it into parts like label: process/pending-review sort: created-asc

The problem then arises that there are many renders of the data, and they all need to do the same url conversion. So we need to have a shortlink-url.jinja2 template in coala-mobans which only renders the data into a url , and it is included in any higher level renderer. Not sure if this is supported by moban yet, but ... muck around a bit, ... and if it doesnt work ... I am very confident that the creator will add the feature once we've clearly defined it.

devops.yml Outdated
- name: commit
url: https://api.coala.io/en/latest/Developers/Writing_Good_Commits.html
- name: cep
url: https://github.com/coala/cEPs/blob/master/cEP-0000.md
Copy link
Member

@jayvdb jayvdb Dec 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

host, repo, branch, filename ?

devops.yml Outdated
- name: channels
url: https://github.com/coala/coala/wiki/Communication-Channels
- name: newform
url: "https://docs.google.com/forms/d/e/1FAIpQLSd7g_MU_c-BMQ62WHeznrvcoXwqW\
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gform: d/e/1FAIpQLSd7g_MU_c-BMQ62WHeznrvcoXwqW87O_Wq4Gz7-pp8PJ38Wdg

devops.yml Outdated
url: https://coala.io/#/gitmate
- name: csoc
url: "https://docs.google.com/forms/d/e/1FAIpQLSeR8WKkZA1R0gBDjJqfeI96jgHe9\
mt8zmAVm1YtB5FpJFD9gQ/viewform?usp=sf_link"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

usp: sf_link

devops.yml Outdated
url: "https://docs.google.com/presentation/d/1qJpXMgA8_WJNO3eUV_ueNiU8w_1fT\
ethNvEuU2XHnWs/edit?usp=sharing"
- name: feedback
url: https://goo.gl/DcTPbn
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expand :P

}

location /newcomer { return 301 https://api.coala.io/en/latest/Developers/Newcomers_Guide.html; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

improve the template so that it adds whitespace to the tab { so that your diff doesnt have any changes. (or at least very few changes)

@yukiisbored yukiisbored force-pushed the yuki_is_bored/shortlinks branch from 4787c74 to d047087 Compare December 20, 2017 08:30
devops.yml Outdated
github_issues: "is:open is:issue user:coala label:difficulty/newcomer\
-label:initiatives/gci no:assignee"
- name: low
github_issues: "is:open is:issue user:coala label:difficulty/low no:assignee"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (81 > 80 characters) (line-length)

Origin: YAMLLintBear, Section: all.yaml.

devops.yml Outdated
- name: low
github_issues: "is:open is:issue user:coala label:difficulty/low no:assignee"
- name: medium
github_issues: "is:open is:issue user:coala label:difficulty/medium no:assignee"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (84 > 80 characters) (line-length)

Origin: YAMLLintBear, Section: all.yaml.

devops.yml Outdated
- name: medium
github_issues: "is:open is:issue user:coala label:difficulty/medium no:assignee"
- name: review
github_pulls: "is:open user:coala label:process/pending review sort:created-asc"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (84 > 80 characters) (line-length)

Origin: YAMLLintBear, Section: all.yaml.

@yukiisbored yukiisbored force-pushed the yuki_is_bored/shortlinks branch 2 times, most recently from f6f9bd8 to c69cb97 Compare December 20, 2017 08:33

{% for sl in shortlinks %}
location /{{ "%-10s" % sl.name }} { return 301 https://{{ get_url(sl) }}; }
{% endfor %}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unlabeled control end tag

Origin: Jinja2Bear, Section: all.jinja2.

The issue can be fixed by applying the following patch:

--- a/tmp/tmp6jl5e9j8/.moban.dt/coala.io.conf.jj2
+++ b/tmp/tmp6jl5e9j8/.moban.dt/coala.io.conf.jj2
@@ -17,5 +17,5 @@
 
     {% for sl in shortlinks %}
     location /{{ "%-10s" % sl.name }} { return 301 https://{{ get_url(sl) }}; }
-    {% endfor %}
+    {% endfor %}{# for sl in shortlinks #}
 }

<td><a href="https://coala.io/{{ sl.name }}">{{ sl.name }}</a></td>
<td><a href="https://{{ get_url(sl) }}">{{ get_url(sl) }}</a></td>
</tr>
{% endfor %}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unlabeled control end tag

Origin: Jinja2Bear, Section: all.jinja2.

The issue can be fixed by applying the following patch:

--- a/tmp/tmp6jl5e9j8/.moban.dt/links.html.jj2
+++ b/tmp/tmp6jl5e9j8/.moban.dt/links.html.jj2
@@ -28,7 +28,7 @@
                     <td><a href="https://coala.io/{{ sl.name }}">{{ sl.name }}</a></td>
                     <td><a href="https://{{ get_url(sl) }}">{{ get_url(sl) }}</a></td>
                 </tr>
-                {% endfor %}
+                {% endfor %}{# for sl in shortlinks #}
             </ul>
         </div>
     </body>

{%- endmacro %}

{% macro gpresentation(id) -%}
docs.google.com/presentation/d/{{id}}/edit
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable blocks should be spaced with 1 spaces on each side.

Origin: Jinja2Bear, Section: all.jinja2.

The issue can be fixed by applying the following patch:

--- a/tmp/tmp6jl5e9j8/.moban.dt/url_builder.jj2
+++ b/tmp/tmp6jl5e9j8/.moban.dt/url_builder.jj2
@@ -19,7 +19,7 @@
 {%- endmacro %}
 
 {% macro gpresentation(id) -%}
-  docs.google.com/presentation/d/{{id}}/edit
+  docs.google.com/presentation/d/{{ id }}/edit
 {%- endmacro %}
 
 {% macro github_issues(q) -%}

{%- endmacro %}

{% macro get_url(sl) %}
{% if sl.api_doc is defined -%}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Control blocks should be spaced with 1 spaces on each side.

Origin: Jinja2Bear, Section: all.jinja2.

The issue can be fixed by applying the following patch:

--- a/tmp/tmp6jl5e9j8/.moban.dt/url_builder.jj2
+++ b/tmp/tmp6jl5e9j8/.moban.dt/url_builder.jj2
@@ -39,7 +39,7 @@
 {%- endmacro %}
 
 {% macro get_url(sl) %}
-  {% if sl.api_doc is defined -%}
+  {% if sl.api_doc is defined - %}
     {{ api_doc(sl.api_doc) }}
   {%- elif sl.user_doc is defined -%}
     {{ user_doc(sl.user_doc) }}

{%- elif sl.user_doc is defined -%}
{{ user_doc(sl.user_doc) }}
{%- elif sl.cEP is defined -%}
{{ cEP(sl.cEP)}}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable blocks should be spaced with 1 spaces on each side.

Origin: Jinja2Bear, Section: all.jinja2.

The issue can be fixed by applying the following patch:

--- a/tmp/tmp6jl5e9j8/.moban.dt/url_builder.jj2
+++ b/tmp/tmp6jl5e9j8/.moban.dt/url_builder.jj2
@@ -44,7 +44,7 @@
   {%- elif sl.user_doc is defined -%}
     {{ user_doc(sl.user_doc) }}
   {%- elif sl.cEP is defined -%}
-    {{ cEP(sl.cEP)}}
+    {{ cEP(sl.cEP) }}
   {%- elif sl.gform is defined -%}
     {{ gform(sl.gform) }}
   {%- elif sl.gpresentation is defined -%}

{%- endmacro %}

{% macro get_url(sl) %}
{% if sl.api_doc is defined -%}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Control start tag has no corresponding end

Origin: Jinja2Bear, Section: all.jinja2.

@yukiisbored yukiisbored force-pushed the yuki_is_bored/shortlinks branch from c69cb97 to 9e3bb69 Compare December 20, 2017 08:37
{%- endmacro %}

{% macro get_url(sl) %}
{% if sl.api_doc is defined -%}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Control blocks should be spaced with 1 spaces on each side.

Origin: Jinja2Bear, Section: all.jinja2.

The issue can be fixed by applying the following patch:

--- a/tmp/tmp3p7yqex9/.moban.dt/url_builder.jj2
+++ b/tmp/tmp3p7yqex9/.moban.dt/url_builder.jj2
@@ -39,7 +39,7 @@
 {%- endmacro %}
 
 {% macro get_url(sl) %}
-  {% if sl.api_doc is defined -%}
+  {% if sl.api_doc is defined - %}
     {{ api_doc(sl.api_doc) }}
   {%- elif sl.user_doc is defined -%}
     {{ user_doc(sl.user_doc) }}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there's a bug with whatever linter we use for jinja2, this is a valid syntax since we don't want it to add the whitespaces after this block.

{%- endmacro %}

{% macro get_url(sl) %}
{% if sl.api_doc is defined -%}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Control start tag has no corresponding end

Origin: Jinja2Bear, Section: all.jinja2.

.moban.yml Outdated
configuration: devops.yml
configuration_dir: ../coala-mobans/
targets:
- "nginx/nginx.conf.d/coala.io.conf": "coala.io.conf.jj2"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need quotes because ... ? the / ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, i think i was looking at configs on pyexcel. let me change that.

- name: newcomers
api_doc: Developers/Newcomers_Guide
- name: new
github_issues: "is:open is:issue user:coala label:difficulty/newcomer\
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ewww.

Copy link
Member

@jayvdb jayvdb Dec 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

github_issues:
  - is:open
  - is:issue
  - user:coala
  - label:difficulty/newcomer
  - "-label:initiatives/gci"
  - no:assignee

- name: viperform
gform: 1FAIpQLSdtdIF5CLnO2erAc41yLRoEvUMXyt3ZWUOVJ5LSqpwZEYF03A
- name: romania
url: "www.bigmarker.com/remote-meetup/Open-Source-and-Google-Summer-of-Code\
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no protocol?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's on the nginx template.

location / {
proxy_pass http://192.30.252.153;
proxy_set_header Host coala.io;
proxy_set_header X-Real-IP $remote_addr;
proxy_intercept_errors on;
}

# URL shortener
location /newcomer { return 302 http://api.coala.io/en/latest/Developers/Newcomers_Guide.html; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nginx doesnt support redirects without a protocol?

if not, can you add a PR first to replace http->https or https->http , so then this diff has only minimal changes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not that nginx doesn't support it. It just throws HTTP 302 with whatever after it. Some programs get confused when there's no protocol.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'll just make some changes to the coala.io.conf so this diff will be lower.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hasnt been done , it appears

@jayvdb
Copy link
Member

jayvdb commented Apr 18, 2018

update mobans repo and rebase...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants