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: add .prettierignore to disable auto-formatting issue across all repositories #253

Merged
merged 9 commits into from
Mar 11, 2024

Conversation

AnimeshKumar923
Copy link
Contributor

Description

@derberg

  • As discussed here, I'm creating a PR to address the issue faced.
  • This file ignores all the auto-formatting used by prettier code extension across different repositories.

cc: @akshatnema @fmvilas @vishvamsinh28

Related issue(s)
Fixes #252

Copy link

@github-actions github-actions bot left a 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.

@AnimeshKumar923 AnimeshKumar923 changed the title fix: add .prettierignore to disable all formatting issue across all repositories fix: add .prettierignore to disable auto-formatting issue across all repositories Jul 17, 2023
@akshatnema
Copy link
Member

@AnimeshKumar923 @derberg Do we have to update any workflow file as well to include this file in updating global workflows?

@AnimeshKumar923
Copy link
Contributor Author

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.

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.

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 🤔

@AnimeshKumar923
Copy link
Contributor Author

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 thinking

@derberg
One suggestion from my side is that we announce it on the contributing channel and ask where to add this file. Can we do this?

@AnimeshKumar923
Copy link
Contributor Author

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

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
and I'm proud (a little bit) of myself to have my first major contribution here. 😊

Copy link
Member

derberg commented Jul 20, 2023

maybe there is a way to use GitHub Search to check in in asyncapi org there is any repo that uses prettier

@AnimeshKumar923
Copy link
Contributor Author

AnimeshKumar923 commented Jul 20, 2023

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 thinking

@derberg One suggestion from my side is that we announce it on the contributing channel and ask where to add this file. Can we do this?

adding to this,
maybe we can start a discussion on this topic on GitHub and let the maintainer join in and discuss.
If required, I can open a PR in every repo that requires the prettierignore file and referencing this original issue from here?
What do you say? @derberg

Copy link
Member

derberg commented Jul 20, 2023

first we should clarify what repo requires it

there are 2 ways:

  • add prettierignore everywhere, excluding repos where prettier is used
  • add prettierignore only to repos that need it

this is what we need to put up for discussion

@vishvamsinh28
Copy link
Contributor

first we should clarify what repo requires it

there are 2 ways:

  • add prettierignore everywhere, excluding repos where prettier is used
  • add prettierignore only to repos that need it

this is what we need to put up for discussion

I think we should go with 2nd option

Copy link
Member

derberg commented Jul 20, 2023

and this is the easiest and least aggressive way definitely

Copy link
Member

@derberg Can we define the list of repositories in the workflow file to specify in which repos we want to replicate this file?

@AnimeshKumar923
Copy link
Contributor Author

AnimeshKumar923 commented Jul 20, 2023

first we should clarify what repo requires it

there are 2 ways:

* add prettierignore everywhere, excluding repos where prettier is used

* add prettierignore only to repos that need it

this is what we need to put up for discussion

I would go with the 2nd option and ask where to add the prettierignore file, which repos require it.
Then create a parent issue containing all the places where it is required and move forward with adding through a PR.

@chinma-yyy
Copy link

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 .prettierrc.* file. If yes then don't add the file to the repo

Copy link
Member

derberg commented Jul 20, 2023

Can we define the list of repositories in the workflow file to specify in which repos we want to replicate this file?

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 get-global-releaserc topic, which means that this repo gets always the latest file whenever it is updated in .github repo. We can have another one, get-global-prettierignore and tell people: hey, add this topic to repo, in a week from now we will add it to .github and then you will get it

@AnimeshKumar923
Copy link
Contributor Author

maybe there is a way to use GitHub Search to check in in asyncapi org there is any repo that uses prettier

Will look into this...

@AnimeshKumar923
Copy link
Contributor Author

Have a look at https://github.com/asyncapi/generator and get-global-releaserc topic, which means that this repo gets always the latest file whenever it is updated in .github repo. We can have another one, get-global-prettierignore and tell people: hey, add this topic to repo, in a week from now we will add it to .github and then you will get it

Hey @derberg can you tell me more about how the topic integrate with providing updates. Any blog/articles on it?

@derberg
Copy link
Member

derberg commented Jul 25, 2023

@AnimeshKumar923 no blog/article, it is our custom solution.

Imagine we push dedicated new configuration for prettier, so it works this way:

  • repo maintainer adds required topic to a repo
  • repo maintainer asks .github repo maintainers to trigger global workflow replication:
    Screenshot 2023-07-25 at 12 52 00
  • bot updates whatever is needed to update

@AnimeshKumar923
Copy link
Contributor Author

@AnimeshKumar923 no blog/article, it is our custom solution.

* we use https://github.com/derberg/manage-files-in-multiple-repositories to replicate files in multiple repos

* one of features is to push files only to repos that have specific topic (more about [gh topics](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/classifying-your-repository-with-topics))

Imagine we push dedicated new configuration for prettier, so it works this way:

* repo maintainer adds required topic to a repo

* repo maintainer asks `.github` repo maintainers to trigger global workflow replication:

* bot updates whatever is needed to update

Let me research about it. Will report you later...

Copy link
Member

derberg commented Jul 26, 2023

but what part you want to research?
someone just needs to ask community in slack, in tooling channel, who wants to get this ignoreprettier file, we give people a week to say they want it and that is it

@vishvamsinh28
Copy link
Contributor

but what part you want to research? someone just needs to ask community in slack, in tooling channel, who wants to get this ignoreprettier file, we give people a week to say they want it and that is it

I'll ask them

@AnimeshKumar923
Copy link
Contributor Author

AnimeshKumar923 commented Jul 26, 2023

but what part you want to research?

@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.

someone just needs to ask community in slack, in tooling channel, who wants to get this ignoreprettier file, we give people a week to say they want it and that is it.

Hmm...okay. I guess @vishvamsinh28 already did ask it here

@akshatnema
Copy link
Member

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 get-global-releaserc topic, which means that this repo gets always the latest file whenever it is updated in .github repo. We can have another one, get-global-prettierignore and tell people: hey, add this topic to repo, in a week from now we will add it to .github and then you will get it

@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.

@AnimeshKumar923
Copy link
Contributor Author

Hi @Shurtu-gal 👋

Need your help in reviewing this. Can you please take a look at it and provide feedback? Thank you!

Copy link
Contributor

@Shurtu-gal Shurtu-gal left a 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

AnimeshKumar923 added a commit to AnimeshKumar923/.github-asyncapi that referenced this pull request Feb 24, 2024
- Added .prettierignore file according to suggestion here: asyncapi#253 (review)
@AnimeshKumar923
Copy link
Contributor Author

AnimeshKumar923 commented Feb 24, 2024

@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 the file, please review it once again... @Shurtu-gal

@Shurtu-gal
Copy link
Contributor

Looks fine to me @AnimeshKumar923, but as I am not the CODEOWNER it would be better to ask for approval of others too.

@AnimeshKumar923
Copy link
Contributor Author

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

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.

please update readme with new topic name

@AnimeshKumar923
Copy link
Contributor Author

please update readme with new topic name

@derberg Did the required change through f5ad226 Please have a look.

NOTE: The URL at L18 of README is currently pointing to my personal fork of this repo. Once this PR is merged, I will update the URL accordingly.

README.md Outdated Show resolved Hide resolved
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.

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

@derberg
Copy link
Member

derberg commented Mar 11, 2024

/rtm

1 similar comment
@derberg
Copy link
Member

derberg commented Mar 11, 2024

/rtm

@asyncapi-bot asyncapi-bot merged commit 3b27869 into asyncapi:master Mar 11, 2024
8 checks passed
@AnimeshKumar923
Copy link
Contributor Author

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

Yes, will do for sure! 👍

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.

add global prettier disable feature
7 participants