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

Annotations in GitLab (pull request proposal) #1990

Open
jord1e opened this issue Aug 20, 2021 · 12 comments
Open

Annotations in GitLab (pull request proposal) #1990

jord1e opened this issue Aug 20, 2021 · 12 comments

Comments

@jord1e
Copy link

jord1e commented Aug 20, 2021

Is your feature request related to a problem? Please describe.

Display annotations in GitLab in addition to standard CI output.

Describe the solution you'd like

Since GitLab version 13.2 (July 2020) Code Quality reports have been made available in the free tier.

The reports are based on the Code Climate issue spec as described in the Implementing a custom tool section.

I would like an option in the graphql-inspector diff command to generate code climate JSON reports.

I propose the following mapping of severity levels:

Criticality Level Code Climate Severity GitHub Reference
DANGEROUS major Warning
NON_BREAKING info Notice
BREAKING critical Failure

Ref: GitHub severity mappings

Icons can be seen on the Vulnerability Report page.

GitLab Code Quality Icons

Example in pull requests:

Code Quality report on pull request page

And in the diff view (only for GitLab ultimate, urghhh):

Code Quality report in the diff view

The JSON for each change will look like the following:

Sample:

{
  criticality: {
    level: 'DANGEROUS',
    reason: 'Adding an enum value may break existing clients that were not programming defensively against an added case when querying an enum.'
  },
  type: 'ENUM_VALUE_ADDED',
  message: "Enum value 'ABC' was added to enum 'EnumA'"
}
{
  "type": "issue",
  "check_name": "ENUM_VALUE_ADDED",
  "description": "Enum value 'ABC' was added to enum 'EnumA'",
  "content": "Adding an enum value may break existing clients that were not programming defensively against an added case when querying an enum.",
  "categories": ["level === 'NON_BREAKING' ? 'Compatibility' : 'Bug Risk'"],
  "fingerprint": "hex(criticality.level + ':' + type + ':' + message)",
  "severity": "major",
  "location": {
    "path": "schema/cool-schema.graphql",
    "lines": {
      "begin": 10,
      "end": 15
    }
  }
}

The option would be --codeclimate "path/filenameString.json" and it would output valid Code Climate JSON to said path.

I would like to contribute this feature via a pull request, but only if the feature gets merged.

Describe alternatives you've considered

Not applicable.

Additional context

Issue #54 and #361


Pinging @kamilkisiela as the only maintainer
@kamilkisiela
Copy link
Owner

@jord1e let's do it :)

@jord1e
Copy link
Author

jord1e commented Sep 27, 2021

@jord1e let's do it :)

Will get started in the next few weeks. It may take some time.

@jord1e
Copy link
Author

jord1e commented Sep 30, 2021

@kamilkisiela would it be okay to add an --format option to the diff command?

graphql-inspector diff old.graphql new.graphql --format gitlab

You'd then redirect the output to a file (using a file pipe >)

I would add a plain and gitlab option

I really don't have the time to recreate the entire GitHub integration (it isn't even possible to do it simply in GitLab).

It would look like this in .gitlab-ci.yml:

graphql-inspector:
  image: <inspector image>
  stage: test
  script:
    - git fetch origin main
    - mkdir -p reports
    - graphql-inspector diff git:origin/main:mygraph.graphql mygraph.graphql --format gitlab > reports/graphql-inspector.json
  artifacts:
    name: '$CI_JOB_NAME artifacts from $CI_PROJECT_NAME on $CI_COMMIT_REF_SLUG'
    expire_in: 1 day
    when: always
    reports:
      codequality:
        - 'reports/*'
    paths:
      - 'reports/*'
  allow_failure:
    exit_codes:
      - 1

The Hadolint project uses the same method, and it works perfectly:
https://github.com/hadolint/hadolint/blob/master/docs/INTEGRATION.md#gitlab-ci

GitLab Hadolint output

Docs will of course be updated to reflect these changes. The default output format would be plain, preserving current behaviour.

@kamilkisiela
Copy link
Owner

@jord1e sounds good!

@jord1e
Copy link
Author

jord1e commented Oct 1, 2021

Is it ok if i move
packages/github/src/helpers/location.ts
to
packages/core/src/utils/location.ts
?

@kamilkisiela
Copy link
Owner

@jord1e yes :)

@jord1e
Copy link
Author

jord1e commented Oct 6, 2021

@kamilkisiela I got this far:
https://gitlab.com/jordiex/graphql-inspector-test/-/merge_requests/2 (MR)
https://gitlab.com/jordiex/graphql-inspector-test/-/jobs/1656530180 (Job output)

It seems to be working great:
afbeelding

Inline hints also seem to be working for free GitLab accounts! Which is also great!
afbeelding

There are a few bugs left (like the undefined in the messages), those will be resolved in the next few days

TODO:

  • Docs (need a native English speaker to check them after)
  • Tests
  • Unify OnComplete handler
  • Fix error description (undefined)

Could you please enable workflows in #2024 ?

Last note:
Can you make this contribution fall under Hacktoberfest? You will need to add the hacktoberfest topic to the repository or tag my PR with hacktoberfest-accepted. This would be greatly appreciated 💚.

I expect to be done early next week.

@kamilkisiela
Copy link
Owner

@jord1e Done :)

@jord1e
Copy link
Author

jord1e commented Oct 7, 2021

I can't figure this out:

I need a Source object here:
https://github.com/kamilkisiela/graphql-inspector/pull/2024/files#diff-c7f995eda48a992dab753a0f8236b5cea24c1fa96176e4730706bb6ec90bd355R151

The Source class is this:

export class Source {
  body: string;
  name: string;
  locationOffset: Location;
  constructor(body: string, name?: string, locationOffset?: Location);
}

The body needs to be the SDL of the schema. The problem is I don't know how to get it there. In the tests 'in-memory schemas' are used (https://github.com/kamilkisiela/graphql-inspector/pull/2024/files#diff-1b58322e0811e8317e77102c70ab98b1ad9871fa9d02d3d92ce8d0b4e64607f6R4)

But the document loading mechanics in the command handler are complicated and seem to use three (?) different The Guild packages. So reading the file contents with readFileSync doesn't work.

@jord1e
Copy link
Author

jord1e commented Dec 17, 2021

Still stuck on that point

@commentatorboy
Copy link

hey @jord1e
Are you still stuck on this?
Because it would be cool to have this out :D

@jord1e
Copy link
Author

jord1e commented Jan 12, 2023

I no longer plan on contributing. You are allowed to fork the fork and keep working with what I already did.

#2024

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

Successfully merging a pull request may close this issue.

3 participants