-
Notifications
You must be signed in to change notification settings - Fork 66
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: add .prettierignore
to disable auto-formatting issue across all repositories
#253
fix: add .prettierignore
to disable auto-formatting issue across all repositories
#253
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
.prettierignore
to disable all formatting issue across all repositories.prettierignore
to disable auto-formatting issue across all repositories
@AnimeshKumar923 @derberg Do we have to update any workflow file as well to include this file in updating global workflows? |
@akshatnema I'm still not so sure about that as I don't understand much where to update the workflows. Will look into it. But it would be better to let @derberg take a look at it and inform about it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the way it is done now means that this file is just for this repo
to have it replicated, you need to add a new job to https://github.com/asyncapi/.github/blob/master/.github/workflows/global-replicator.yml
something like this one: https://github.com/asyncapi/.github/blob/master/.github/workflows/global-replicator.yml#L91-L107
and I think we should not push to all by default, but probably agree on some github repo topic name that repo maintainer can add to get this file, maybe 🤔
@derberg |
Hmmm...I'll have to look into that as I'm unfamiliar with YAML files and workflows stuff, but got some experience while working with this: asyncapi/community#805 so give me some time to figure this out... @derberg |
maybe there is a way to use GitHub Search to check in in asyncapi org there is any repo that uses prettier |
adding to this, |
first we should clarify what repo requires it there are 2 ways:
this is what we need to put up for discussion |
I think we should go with 2nd option |
and this is the easiest and least aggressive way definitely |
@derberg Can we define the list of repositories in the workflow file to specify in which repos we want to replicate this file? |
I would go with the 2nd option and ask where to add the |
What if we add a condition on the file replicator workflow to check if the repository has any prettier config file? We must check if the repo has a |
we can provide a list of repos, but we also can specify a github topic that one can add to repo, easier than opening a PR to add new repo to the list. Have a look at https://github.com/asyncapi/generator and |
Will look into this... |
Hey @derberg can you tell me more about how the topic integrate with providing updates. Any blog/articles on it? |
@AnimeshKumar923 no blog/article, it is our custom solution.
Imagine we push dedicated new configuration for prettier, so it works this way: |
Let me research about it. Will report you later... |
but what part you want to research? |
I'll ask them |
@derberg By research I mean to have a look at the custom method you mentioned. I'll study about it a bit because this is new to me. I'm having some college work lately, so getting less time to devote here. Sorry about that, but will do the needful from my side.
Hmm...okay. I guess @vishvamsinh28 already did ask it here |
@derberg I'm good with this approach. We just have to ask the maintainers of all repos to add the tag in their repo, if they want this file to be added. |
Hi @Shurtu-gal 👋 Need your help in reviewing this. Can you please take a look at it and provide feedback? Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AnimeshKumar923 I couldn't see any .prettierignore
file which needs to be replicated.
You should add one like in https://github.com/asyncapi/.github/blob/master/.github/workflows/.releaserc
- Added .prettierignore file according to suggestion here: asyncapi#253 (review)
Added the file, please review it once again... @Shurtu-gal |
Looks fine to me @AnimeshKumar923, but as I am not the CODEOWNER it would be better to ask for approval of others too. |
Thank you so much for the help! Appreciate it. @Shurtu-gal Bringing your kind attention to this @derberg |
- Added .prettierignore file according to suggestion here: asyncapi#253 (review)
14c8d4e
to
242366b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please update readme with new topic name
minor url fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Would be great if after merging this PR you could drop a message in tooling channel, for maintainers that they can use new topic to get the global prettier to protect from PRs that apply auto formatting
/rtm |
1 similar comment
/rtm |
Yes, will do for sure! 👍 |
Description
@derberg
cc: @akshatnema @fmvilas @vishvamsinh28
Related issue(s)
Fixes #252