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

Tools: Husky upgrade and git ignorance improved #33183

Merged
merged 5 commits into from
Jul 6, 2021
Merged

Tools: Husky upgrade and git ignorance improved #33183

merged 5 commits into from
Jul 6, 2021

Conversation

shivapoudel
Copy link
Contributor

Description

Types of changes

Just a development files changes.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Jul 3, 2021
@github-actions
Copy link

github-actions bot commented Jul 3, 2021

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @shivapoudel! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

@gziolo gziolo added the [Type] Build Tooling Issues or PRs related to build tooling label Jul 5, 2021
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

I have only one concern regarding the changes in .gitattributes. Otherwise it looks like Husky v7 fixed the issue with _ subfolder 👍🏻

Thank you for the follow-up on the Husky integration 😄

.gitattributes Outdated Show resolved Hide resolved
@gziolo gziolo changed the title Husky upgrade and git ignorance improved Tools: Husky upgrade and git ignorance improved Jul 6, 2021
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

Nice one, thank you for your contribution 💯

@gziolo
Copy link
Member

gziolo commented Jul 6, 2021

It looks like vendor/.gitignore ensures that the vendor folder exists while running the script that builds the zip file for the Gutenberg plugin:

Screen Shot 2021-07-06 at 11 27 19

There might be more places related to vendor scripts that assume that the vendor folder exists.

It's very unfortunate, but I think it's simpler to leave it as is for now to unblock this PR.

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

See above.

@shivapoudel
Copy link
Contributor Author

@nylen As I am aware of vendor directory is utilized by Composer dependencies but it is used to download vendor JS files through this PR #1025

@gziolo If this vendor JS files can be in tmp, dist or somewhere else that would be nice in my opinion!

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

All green, let's land it 🎉

@gziolo gziolo merged commit 2c822ac into WordPress:trunk Jul 6, 2021
@github-actions
Copy link

github-actions bot commented Jul 6, 2021

Congratulations on your first merged pull request, @shivapoudel! We'd like to credit you for your contribution in the post announcing the next WordPress release, but we can't find a WordPress.org profile associated with your GitHub account. When you have a moment, visit the following URL and click "link your GitHub account" under "GitHub Username" to link your accounts:

https://profiles.wordpress.org/me/profile/edit/

And if you don't have a WordPress.org account, you can create one on this page:

https://login.wordpress.org/register

Kudos!

@github-actions github-actions bot added this to the Gutenberg 11.1 milestone Jul 6, 2021
@gziolo
Copy link
Member

gziolo commented Jul 6, 2021

If this vendor JS files can be in tmp, dist or somewhere else that would be nice in my opinion!

Good observation, maybe build/vendor/ would be a better location, but it requires some changes in how those scripts are downloaded and consumed.

sarayourfriend pushed a commit that referenced this pull request Jul 15, 2021
* With husky 7.0.0, .husky/.gitignore is now unnecessary and can be removed
Ref: #32077 (comment)
CC @gziolo

* Declare files that will always have LF line endings on checkout

* Git ignore Windows specific files and remove composer vendor dir

* Remove special handling for PHP files from .gitattributes

* Unfortunately vendor dir is required to pass the test

Co-authored-by: Greg Ziółkowski <grzegorz@gziolo.pl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Type] Build Tooling Issues or PRs related to build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants