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

Feat: Add shared danger file to ipfs repos #69

Open
SgtPooki opened this issue Sep 21, 2022 · 2 comments
Open

Feat: Add shared danger file to ipfs repos #69

SgtPooki opened this issue Sep 21, 2022 · 2 comments

Comments

@SgtPooki
Copy link
Member

SgtPooki commented Sep 21, 2022

Context: In awesome-ipfs, I added https://github.com/ipfs/awesome-ipfs/blob/master/dangerfile.js which can help to block and give more information in a comment on PRs. I talked with @lidel about this and we agreed that this would be a good addition to many of our repos.

Danger currently only has official runners for JS, ruby, swift, python, and kotlin, but there is the possibility to create a danger-runner in any language (Go, Rust, etc..): https://danger.systems/js/usage/danger-process.html
The ask for this issue is that we come up with a base file that would work for multiple repos to use as a default. Some default rules we could use are:

  1. Warning on large PRs (over X lines changed)
  2. Fail when src/ is modified without test/ modified
  3. Display lint/test failures
  4. Display code coverage changes (This should probably be done via codecov)

And there are other examples and plugins listed at https://danger.systems/js/


A few issues I could see with implementing this in github-mgmt:

  1. Accidentally overwriting a customized danger file
  2. Setting rules that are too strict for some repos

So I guess the main goal here is to make it easy for ipfs repos to get a danger file that works, and then be allowed to modify it for their needs as appropriate.

@SgtPooki
Copy link
Member Author

There is a large example with some fairly useful stuff at https://github.com/apollographql/apollo-client/blob/6d49d993c6cf0eb0fcf04b0a718a8be2ada3c542/config/dangerfile.ts, though they have since removed it from their repo

@SgtPooki
Copy link
Member Author

example improvement that could be made to ipfs/awesome-ipfs to give helpful hints in PRs:

const { danger, markdown, fail, warn, message } = require('danger')

danger.git.modified_files.forEach(file => {
  danger.git.diffForFile(file).then(diff => {
    if (diff == null) {
      return
    }

    if (file.includes('data/')) {
      let hasTitle = false
      let hasDescription = false
      let hasImage = false
      let hasSource = false
      const entries = []
      diff.added.split('\n').forEach(line => {
        if (line.includes('picture:')) {
          hasImage = true
          // check that the image exists
          const picturePath = line.split('picture:')[1].trim()
          if (!fs.existsSync(`src/static/images/${picturePath}`)) {
            fail(`The image ${picturePath} does not exist`)
          }
        } else if (line.includes('title:')) {
          hasTitle = true
        } else if (line.includes('description:')) {
          hasDescription = true
        } else if (line.includes('source:')) {
          hasSource = true
        }
      })
      /**
       * Ensure that entries to data/ contain a
       *  1. title
       *  2. source
       *  3. description
       */
      if (!diff.added.includes('title:')) {
        fail(`Please add a title to ${file}`)
      }
      if (!diff.added.includes('source:')) {
        fail(`Please add a source to ${file}`)
      }
      if (!diff.added.includes('description:')) {
        fail(`Please add a description to ${file}`)
      }
      /**
       * Ensure that http links use https
       */
      if (diff.added.includes('http://')) {
        fail(`Please use https:// instead of http:// in ${file}`)
      }
    }
  })
})

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

No branches or pull requests

1 participant