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: updated tools workflow #1627

Merged
merged 7 commits into from
Jul 24, 2023
Merged

Conversation

akshatnema
Copy link
Member

Description
Updated tools inside the website and refactored the code of the tools script.

Related issue(s)
Fixes #1604

@netlify
Copy link

netlify bot commented May 4, 2023

Deploy Preview for asyncapi-website ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 876e9a4
🔍 Latest deploy log https://app.netlify.com/sites/asyncapi-website/deploys/64be8b1ce18fb800077c483d
😎 Deploy Preview https://deploy-preview-1627--asyncapi-website.netlify.app
📱 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 configuration.

Comment on lines 78 to 92
{
"title": "Golang AsyncAPI Code Generator",
"description": "Generate Go client and server boilerplate from AsyncAPI specifications. Can be called from `go generate` without requirements.\n",
"links": {
"repoUrl": "https://github.com/lerenn/asyncapi-codegen"
},
"filters": {
"language": "golang",
"categories": [
"code-generator"
],
"hasCommercial": false,
"isAsyncAPIOwner": false
}
},
Copy link
Member Author

Choose a reason for hiding this comment

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

@derberg Many of the tools like this are no more recognizable from GitHub code search. I don't know why but for every GET call of API, there are only 14 results.

Copy link
Member

Choose a reason for hiding this comment

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

for now just update in the way to make sure it is not removed and we can merge I guess

@github-actions
Copy link

github-actions bot commented May 4, 2023

⚡️ Lighthouse report for the changes in this PR:

Category Score
🔴 Performance 37
🟢 Accessibility 98
🟢 Best practices 100
🟢 SEO 100
🔴 PWA 30

Lighthouse ran on https://deploy-preview-1627--asyncapi-website.netlify.app/

@@ -1,4 +1,5 @@
const axios = require('axios');
require('dotenv').config()
Copy link
Member

Choose a reason for hiding this comment

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

not needed right?

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 needed because I whenever this script in local, I've to replace the process.env with my token there. This file is not part of next js folders so it doesn't take the variables from .env or .env.local by default.

Copy link
Member

Choose a reason for hiding this comment

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

but we do not have .env in website, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, this file is part of .gitignore, but we need some env variables to run the scripts or run Netlify functions. So, this file shouldn't be part of GitHub repo.

Copy link
Member

Choose a reason for hiding this comment

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

but these are already in Netlify
sorry but I really don't get it why we need i here. I know people use .env if they have a lot of env variables that they need to pass locally to run some stuff. But we do not have it right? and no need to read it?

Copy link
Member

Choose a reason for hiding this comment

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

@akshatnema pingy pongo

Copy link
Member Author

Choose a reason for hiding this comment

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

but these are already in Netlify

These files are not part of Netlify or Next js build, so they won't be able to take .env or .env.local files from the directory for local environment setup.

We also know that tools workflow file run in local dev setup only, not in Github actions workflow, so to provide it with proper env setup, we can add this line to get the variables from .env file. Also, it won't affect any changes or usage inside Github actions as well.

@akshatnema akshatnema requested a review from Mayaleeeee as a code owner July 21, 2023 16:49
@akshatnema
Copy link
Member Author

akshatnema commented Jul 21, 2023

@derberg I recently checked with the Github API that they have improved their Github Search API results and it's working fine with correct results. Hence, I will enable the workflow inside website repository after this PR so that we don't have to do this step manually.

Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

NICE!

@akshatnema
Copy link
Member Author

/rtm

@asyncapi-bot asyncapi-bot merged commit ae9048e into asyncapi:master Jul 24, 2023
@akshatnema akshatnema deleted the tools-update branch October 30, 2023 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Looks like tools dashboard script is not reading technology accurately
3 participants