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

Clean up some alerts detected by LGTM [pypi] #2010

Merged
merged 2 commits into from
Aug 29, 2018
Merged

Clean up some alerts detected by LGTM [pypi] #2010

merged 2 commits into from
Aug 29, 2018

Conversation

paulmelnikow
Copy link
Member

https://lgtm.com/projects/g/badges/shields/alerts/?mode=list

To help us keep up to date on this, We could consider adding the readme badge and/or the GitHub integration.

@paulmelnikow paulmelnikow changed the title Clean up some alerts detected by LGTM Clean up some alerts detected by LGTM [pypi] Aug 29, 2018
@shields-ci
Copy link

Messages
📖

✨ Thanks for your contribution to Shields, @paulmelnikow!

Generated by 🚫 dangerJS

@tooomm
Copy link
Contributor

tooomm commented Aug 29, 2018

Which WIP app did you add to the repo?
It seems to be broken or stuck on PR titles which contain squared brackets. I happily report this if you let me know which service it is @paulmelnikow.


Edit:
Hmm, but in #1838 it worked.
While it is stuck in #1793 with the same Expected — Waiting for status to be reported error message.

And in #1205 it's still Pending — work in progress.

@paulmelnikow
Copy link
Member Author

It's this one: https://github.com/apps/wip

Yea, I've noticed it sometimes shows the wrong status.

@@ -8,7 +8,7 @@ const isPsycopg2Version = Joi.string().regex(/^v([0-9][.]?)+$/)

// These regexes are the same, but declared separately for clarity.
const isPipeSeparatedPythonVersions = Joi.string().regex(
/^([0-9]+.[0-9]+(?: \| )?)+$/
/^([0-9]+\.[0-9]+(?: \| )?)+$/
Copy link
Member

@PyvesB PyvesB Aug 29, 2018

Choose a reason for hiding this comment

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

Nice catch! 👍

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 a really helpful service!

@paulmelnikow paulmelnikow merged commit f8f2d88 into badges:master Aug 29, 2018
@shields-deployment
Copy link

This pull request was merged to master branch. This change is now waiting for deployment, which will usually happen within a few days. Stay tuned by joining our #ops channel on Discord!

After deployment, changes are copied to gh-pages branch:

@paulmelnikow paulmelnikow deleted the clean-lgtm branch August 29, 2018 20:37
@paulmelnikow
Copy link
Member Author

@samlanning I'm curious to see if this change fixed the alerts. Is there a way I can trigger https://lgtm.com/projects/g/badges/shields/ to update using the latest build?

@s0
Copy link
Contributor

s0 commented Aug 29, 2018

@paulmelnikow I've given LGTM a poke to fetch and produce results for the latest version of your code, it should appear soon hopefully (it's usually once every 24 hours).

Of course if you enable pull request integration, you'll be able to see how a PR would affect alerts immediately (before merging even) 😉

@s0
Copy link
Contributor

s0 commented Aug 29, 2018

@paulmelnikow
Copy link
Member Author

Looks like I got 3/4 right!

@s0
Copy link
Contributor

s0 commented Aug 30, 2018

@paulmelnikow in fairness, the message for the one you didn't fix did change, it's still triggered by the same query, but it's pointing out something different now :)

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

Successfully merging this pull request may close these issues.

5 participants