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

Document lint codes #2486

Closed
carolynvs opened this issue Dec 6, 2022 · 5 comments · Fixed by #2493
Closed

Document lint codes #2486

carolynvs opened this issue Dec 6, 2022 · 5 comments · Fixed by #2493
Labels
docs Markdown ahoy! Updates needed on porter.sh or in dev docs. help wanted Good for someone who has contributed before

Comments

@carolynvs
Copy link
Member

When a user runs porter lint, there is a bunch of metadata returned for each result, including a URL that explains the problem in more detail.

Let's create a page on the Porter website that documents all known lint messages for Porter (not specific mixins).

  1. Create a new markdown file in docs/content/reference/linter.md. It helps to copy an existing markdown file and then edit it.
  2. Set the page title to "Porter Lint Messages" and include a short introduction that explains that these are messages that may be returned from the porter lint command (you can link to the command, it's located at /cli/porter_lint/).
  3. Document each porter lint message (right now we just have porter-100 for the reserved parameter prefix) in its own separate heading so that we can link directly to the lint message with a URL, like https://getporter.org/reference/linter/#porter-100.
  4. Update existing lint messages and set the URL field to the link from the previous step so that when people encounter the message, they can view the webpage.

See our Contributing Tutorial and New Contributor Guide for help getting started contributing to Porter.

@carolynvs carolynvs added help wanted Good for someone who has contributed before docs Markdown ahoy! Updates needed on porter.sh or in dev docs. labels Dec 6, 2022
@carolynvs carolynvs mentioned this issue Dec 6, 2022
4 tasks
@0xquark
Copy link
Contributor

0xquark commented Dec 7, 2022

Hi @carolynvs 👋 , I would like to pick up this issue. Could you brief me more about this?

@carolynvs
Copy link
Member Author

Hey @0xquark, I recommend first going through the contributor tutorial which will give you an understanding of how to use porter, and then walk you through making changes to the code.

https://getporter.org/quickstart

We only have a single lint error defined so far, exec-100, which can be reproduced with the following:

  1. make a new bundle by running porter create in an empty directory

  2. edit the porter.yaml file and change the install section to the code below, this will trigger the lint error

    install:
    - exec:
        description: such quotes, much wow
        command: bash
        flags:
          c: "echo Hello World"
  3. Run porter lint and you should see the following output

$ porter lint
Running linters for each mixin used in the manifest...
warning(exec-100) - Best Practice: Avoid Embedded Bash
install: 1st step in the exec mixin (such quotes, much wow)
See https://getporter.org/best-practices/exec-mixin/#use-scripts for more information
---
error(exec-101) - bash -c argument missing wrapping quotes
install: 1st step in the exec mixin (such quotes, much wow)
The bash -c flag argument must be wrapped in quotes, for example
exec:
  description: Say Hello
  command: bash
  flags:
    c: '"echo Hello World"'

See https://getporter.org/best-practices/exec-mixin/#quoting-escaping-bash-and-yaml for more information
---

Follow the changes described in the issue to document the lint error and then you can preview your documentation changes either by submitting a PR and looking at the netlify build check, or by running the documentation locally.

@0xquark
Copy link
Contributor

0xquark commented Dec 9, 2022

Hi , So i am able to set up the porter on machine using the tutorial, tinkered around it and was able to reproduce this lint error.So now i am documenting the error , I wanted to know two things before :

  1. I am not able to locate /cli/porter_lint/ . Could you refer the full path ? [https://github.com/getporter/porter/blob/main/cli/porter_lint] doesn't exist
  2. I just wanted to make sure i understand the error right.Porter reserves the porter- or porter_ prefix . If you use this prefix for a parameter in your bundle, you may encounter issues when running Porter lint.
    I am not sure about this in above case as the error was due to escaping bash characters.
    exec-100 doesn't show the reserved parameter as error but i am able to understand that it was due to using - prefix.

@carolynvs
Copy link
Member Author

I was referring to the lint command documentation located at https://getporter.org/cli/porter_lint/.

Right now we just have exec-100 in Porter, which has to do with embedding bash commands in porter.yaml. The other message, porter-100 (reserved parameter prefix for porter), is still in development in an open pull request and would be reproduced differently. Let's just do exec-100 and the page that you create can be updated when the other PR is merged.

@0xquark
Copy link
Contributor

0xquark commented Dec 9, 2022

I was referring to the lint command documentation located at https://getporter.org/cli/porter_lint/.

Right now we just have exec-100 in Porter, which has to do with embedding bash commands in porter.yaml. The other message, porter-100 (reserved parameter prefix for porter), is still in development in an open pull request and would be reproduced differently. Let's just do exec-100 and the page that you create can be updated when the other PR is merged.

Thanks! I just got confused :/ Now i'll be documenting it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Markdown ahoy! Updates needed on porter.sh or in dev docs. help wanted Good for someone who has contributed before
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants