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

fix: use sponsor's GitHub username when name is missing #392

Merged
merged 5 commits into from Dec 25, 2022

Conversation

chenxsan
Copy link
Contributor

@chenxsan chenxsan commented Dec 24, 2022

Prerequisites checklist

What is the purpose of this pull request?

The CI has failed because of a null value of name coming from here 415e0bb#diff-ff108750031135fb05819cca15c41069cf217df9c9e21745012a5af2e965c55bR256.

And I can confirm it's not a glitch or something because the Web version of GitHub GraphQL API did return null for this sponsor:

CleanShot 2022-12-24 at 19 38 49@2x

What changes did you make? (Give an overview)

  1. Set a fallback value for name in template.

    Currently I'm setting it to '', but I'm not sure it's a good option as it could mess the layout:

    But it shouldn't be a problem once the data fetching is done next midnight because there's another fallback value in fetch-sponsors.js, see next.

  2. Set the value of name to login when null in fetch-sponsors.js

Related Issues

Is there anything you'd like reviewers to focus on?

@netlify
Copy link

netlify bot commented Dec 24, 2022

👷 Deploy request for es-eslint pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 089ba10

@eslint-github-bot eslint-github-bot bot added bug Something isn't working triage labels Dec 24, 2022
@netlify
Copy link

netlify bot commented Dec 24, 2022

Deploy Preview for new-eslint ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 089ba10
🔍 Latest deploy log https://app.netlify.com/sites/new-eslint/deploys/63a7ada103f6990008ddf81f
😎 Deploy Preview https://deploy-preview-392--new-eslint.netlify.app/sponsors
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Dec 24, 2022

Deploy Preview for ja-eslint ready!

Name Link
🔨 Latest commit 089ba10
🔍 Latest deploy log https://app.netlify.com/sites/ja-eslint/deploys/63a7ada15ceaef00081ad9f2
😎 Deploy Preview https://deploy-preview-392--ja-eslint.netlify.app/sponsors
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Dec 24, 2022

Deploy Preview for zh-hans-eslint ready!

Name Link
🔨 Latest commit 089ba10
🔍 Latest deploy log https://app.netlify.com/sites/zh-hans-eslint/deploys/63a7ada1e6fcdf0008c558fe
😎 Deploy Preview https://deploy-preview-392--zh-hans-eslint.netlify.app/sponsors
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Dec 24, 2022

Deploy Preview for fr-eslint ready!

Name Link
🔨 Latest commit 089ba10
🔍 Latest deploy log https://app.netlify.com/sites/fr-eslint/deploys/63a7ada11e07e80008db417e
😎 Deploy Preview https://deploy-preview-392--fr-eslint.netlify.app/sponsors
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Dec 24, 2022

Deploy Preview for hi-eslint ready!

Name Link
🔨 Latest commit 089ba10
🔍 Latest deploy log https://app.netlify.com/sites/hi-eslint/deploys/63a7ada1f8849b0009b933a6
😎 Deploy Preview https://deploy-preview-392--hi-eslint.netlify.app/sponsors
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Dec 24, 2022

Deploy Preview for pt-br-eslint ready!

Name Link
🔨 Latest commit 089ba10
🔍 Latest deploy log https://app.netlify.com/sites/pt-br-eslint/deploys/63a7ada13e78b30008405165
😎 Deploy Preview https://deploy-preview-392--pt-br-eslint.netlify.app/sponsors
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Dec 24, 2022

Deploy Preview for de-eslint ready!

Name Link
🔨 Latest commit 089ba10
🔍 Latest deploy log https://app.netlify.com/sites/de-eslint/deploys/63a7ada1601b73000802b5ce
😎 Deploy Preview https://deploy-preview-392--de-eslint.netlify.app/sponsors
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@chenxsan
Copy link
Contributor Author

As you can see from the actions, CI no longer fails like #391

@chenxsan
Copy link
Contributor Author

chenxsan commented Dec 25, 2022

Here's an update of the latest changes:

  1. Use sponsor.login as the fallback value for sponsor.name in case it's null, there're two places in fetch-sponsors.js.
  2. Clean up dirty data in sponsors.json and donations.json to fix the CI, the latest data will be fetched at next midnight according to data-fetch.yml .

@mdjermanovic mdjermanovic changed the title fix: set fallback value fix: use sponsor's GitHub username when name is missing Dec 25, 2022
@mdjermanovic
Copy link
Member

I thought that we remove the entry from donations.json (as that's causing build failures) while keeping sponsors.json the same until the next automated fetch which will replace nulls with usernames. Can you revert sponsors.json? If not, I'll merge as-is, since the entries will get back in a few hours anyways.

Copy link
Member

@amareshsm amareshsm left a comment

Choose a reason for hiding this comment

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

LGTM

@amareshsm
Copy link
Member

I thought that we remove the entry from donations.json (as that's causing build failures) while keeping sponsors.json the same until the next automated fetch which will replace nulls with usernames. Can you revert sponsors.json? If not, I'll merge as-is, since the entries will get back in a few hours anyways.

we can merge this.

Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@mdjermanovic mdjermanovic merged commit 17a9cd8 into eslint:main Dec 25, 2022
@chenxsan chenxsan deleted the bugfix/fix-ci branch December 26, 2022 05:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants