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

Support for multibranch pipelines #55

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

jitbasemartin
Copy link

Hello,

I add the feature mentioned in #16. I use it on my vscode

PS: Be nice, it's my first PR on Github

@alefragnani
Copy link
Owner

Hi @jitbasemartin ,

First of all, thank you for your PR 👍

I don't use multibranch, neither know it works, so it's great to see someone contributing to this.

Since you said to be your first PR on GitHub, I would recommend you the same thing I was recommended on my first PR. Don't change coding standard/format on your PRs. I guess it wasn't intentional, but I noted a few changes which just do that, so I would ask you to revert those changes and push an update to the PR.

I will try to take a look at this next week.

Thank you

@alefragnani alefragnani self-assigned this Aug 16, 2020
@alefragnani alefragnani self-requested a review August 16, 2020 16:16
@jitbasemartin
Copy link
Author

Hello @alefragnani
I'm not sure to understand about coding standard, can you show me an example ?
If it's about white space and other stuff, I'm use to auto fix with eslint. Can you provide your coding style rules ?

@alefragnani
Copy link
Owner

Hi @jitbasemartin ,

Yes, exactly the white space stuff. Take a look on the differences in src/Jenkins.ts file, for instance.

I use ESLint too, and the rules are defined inside package.json, on a shared config I created for my extensions:

    "eslintConfig": {
      "extends": [
        "vscode-ext"
      ]
    },

In the end, it is basically the recommended rules for ESLint and TypeScript, with minor tweaks.

    extends: [
        "eslint:recommended",
        "plugin:@typescript-eslint/recommended",
    ],
    rules: {
        "@typescript-eslint/explicit-module-boundary-types": 0,
        "@typescript-eslint/no-unused-vars": 1,
        "@typescript-eslint/no-explicit-any": 1,
        "@typescript-eslint/no-non-null-assertion": 1,
        "eqeqeq": 2
    }

But it appears more than just ESLint, like a formatter. Are you using Prettier or something similar?

@alefragnani alefragnani added enhancement good first issue (PR available) PR available for that issue PR needs update The PR needs review by the author labels Aug 18, 2020
@alefragnani alefragnani added this to the 4.3.0 milestone Aug 18, 2020
@alefragnani alefragnani linked an issue Aug 18, 2020 that may be closed by this pull request
@alefragnani alefragnani removed this from the October 2021 milestone Nov 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement good first issue (PR available) PR available for that issue PR needs update The PR needs review by the author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for multibranch pipelines
2 participants