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

Added support for chatops using prettier and github actions #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Added support for chatops using prettier and github actions #37

wants to merge 1 commit into from

Conversation

tushar5526
Copy link
Member

Closes #35
This PR adds a workflow which uses prettier to auto-format the files with js css vue jsx files
Steps Followed :

Initialized npm in the repo
Added .gitignore to ignore node-modules
Added the workflow in prettier.yml inside .github/workflows
Formatting will happen only when we use /style in conversation of a PR

@tushar5526
Copy link
Member Author

I saw some blogs, where we can put more features in this. Like putting a reaction ( emoji ) after jobs complete and admin-level rights to call the jobs.
Should I try working on this ? @rahul799 @und3fined-v01d

@rahul799
Copy link
Member

Ya... You can... I will suggest that can you open a issue for that, maybe others who are interested can also work on it.

repo-token: ${{ secrets.GITHUB_TOKEN }}
- uses: r-lib/actions/setup-r@master
- name: Install
run: yarn run prettier --write '**/*.{css,js,vue}'
Copy link
Member

Choose a reason for hiding this comment

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

right now we won't have .vue type file. So you can remove it. Also, you must add .html.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure 👍

Comment on lines 11 to 14
- id: file_changes
uses: trilom/[email protected]
- name: testing
run: echo '${{ steps.file_changes.outputs.files_modified}}'
Copy link
Member

Choose a reason for hiding this comment

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

I won't think we need this as you are running prettier in all the files.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, I will look into this step.
Thanks for pointing out

Copy link
Member

@rahul799 rahul799 left a comment

Choose a reason for hiding this comment

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

Please make the changes.

package.json Outdated
Comment on lines 1 to 24
{
"name": "simple-static-website",
"version": "1.0.0",
"description": "This repository will help beginners who have just completed the Git course & want to apply their theoretical knowledge.",
"main": "index.js",
"dependencies": {
"prettier": "2.0.5"
},
"devDependencies": {},
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"repository": {
"type": "git",
"url": "git+https://github.com/tushar5526/simple-static-website.git"
},
"keywords": [],
"author": "",
"license": "ISC",
"bugs": {
"url": "https://github.com/tushar5526/simple-static-website/issues"
},
"homepage": "https://github.com/tushar5526/simple-static-website#readme"
}
Copy link
Member

Choose a reason for hiding this comment

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

do we need this file?

Copy link
Member Author

Choose a reason for hiding this comment

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

I am not well versed with node.js, but prettier was giving up errors. So I installed it before hand, but I can try a work around in which I install prettier at run time and add files by their extensions, instead of adding all files using git add .

yarn.lock Outdated
Comment on lines 1 to 8
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1


[email protected]:
version "2.0.5"
resolved "https://registry.yarnpkg.com/prettier/-/prettier-2.0.5.tgz#d6d56282455243f2f92cc1716692c08aa31522d4"
integrity sha512-7PtVymN48hGcO4fGjybyBSIWDsLU4H4XlvOHfq91pz9kkGlonzwTfYkaIEwiRg/dAJF9YlbsduBAgtYLi+8cFg==
Copy link
Member

Choose a reason for hiding this comment

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

same goes for this

@tushar5526
Copy link
Member Author

take a look now @rahul799 , I am now installing yarn at run time

Working demo : link

PS: there is one HTML file that is "faulty" in the repo, prettier is showing some errors on it link

Soham also asked to include jsx files but we can make it as an issue ?

Formatting will happen only when we use /style in conversation of a PR

Currently, it formats the files with .js .html and .css extensions
@rahul799
Copy link
Member

right now we won't have JSX files in the repo. So it will have no significance.

@tushar5526
Copy link
Member Author

Any other changes @rahul799 ?

@rahul799
Copy link
Member

@tushar5526
Copy link
Member Author

tushar5526 commented Jun 18, 2020

hey @rahul799 index.html has some errors see this That's why I had excluded .html files earlier to test if prettier is working fine or not

See the Run Yarn section in the above link
Line 221 in index.html has closing tag missing

@rahul799
Copy link
Member

rahul799 commented Jun 18, 2020

It will be good if you debug that.
Hint : you just need to delete the extra h2 tag, as shown in the error log.

@rahul799
Copy link
Member

ok let us leave it for now. I think we are good to go.

@rahul799 rahul799 requested a review from sovoid June 18, 2020 10:57
@tushar5526
Copy link
Member Author

tushar5526 commented Jun 18, 2020

I have changed it in sandbox website I have, and made an issue for someone else to fix it here.`
span tag is missing in index.html

@tushar5526
Copy link
Member Author

Job built successfully link

@rahul799
Copy link
Member

rahul799 commented Jun 18, 2020 via email

@tushar5526
Copy link
Member Author

Looking for next tasks :P @rahul799

@rahul799
Copy link
Member

For now, we are good to merge it.

@tushar5526
Copy link
Member Author

I am thinking to work on #38 , should I add commits in this PR or wait for it to merge ? @rahul799 @und3fined-v01d

@sovoid
Copy link

sovoid commented Jun 22, 2020

@tushar5526. I think its better to wait till this gets merged. Especially since we might transition the entire code here as a config to hydrabot. Wait for the PR to be merged and then we can create a separate for the remainder

@tushar5526
Copy link
Member Author

Sure @und3fined-v01d

@rahul799
Copy link
Member

@und3fined-v01d so are you going to merge it?

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.

Make a Github action bot that automatically does the file formatting.
3 participants