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.

Sign up for GitHub

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

Jump to bottom

Handling nested directories #1

Closed
ekerstein opened this issue Mar 31, 2020 · 3 comments · Fixed by #2
Closed

Handling nested directories #1

ekerstein opened this issue Mar 31, 2020 · 3 comments · Fixed by #2
Assignees
@nizarmah
Labels
bug Something isn't working enhancement New feature or request good first issue Good for newcomers

Comments

@ekerstein
Copy link

Thank you for making this! How do you recommend handling multiple directories? For example if I have:

js/a
js/b
js/c

Can I do something like directory: 'js/*'?
@nizarmah
Copy link
Owner

Hello! Thank you for using this Action ( :

The goal is to use directory: js/* in order to handle multiple directories. However, I need to make sure that it works, as sometimes I have small bugs in my code sadly.

Give me some time (about 10 mins), and I will update you on whether it works or not.

@nizarmah nizarmah self-assigned this Mar 31, 2020
@nizarmah nizarmah added help wanted Extra attention is needed question Further information is requested labels Mar 31, 2020
@nizarmah
Copy link
Owner

nizarmah commented Mar 31, 2020 •
edited
Loading

Alright, so according to this commit, it seems like the library does not support any * operator after the path.

This has become an issue now / bug report

However, I do understand how inefficient it would be to go through the following directories:

js/a
js/b
js/c

So, I have 3 solutions that I am thinking of, and they are:
1- Add a possibility for a * operator that would go through all the js directory and all the directories inside.
2- Add a recursive option recursive: true in order to target the directories inside js.
3- Add a sub-directories array that would target all the directories specified inside js.

I believe the most logical would be the first approach. Then the directory could be treated similarly to how the find command on linux would.

It shouldn't be difficult to apply that solution, it is just a matter of pipelining the output of one to the other. I'll keep you updated ( :

@nizarmah nizarmah added bug Something isn't working enhancement New feature or request good first issue Good for newcomers and removed help wanted Extra attention is needed question Further information is requested labels Mar 31, 2020
@nizarmah nizarmah modified the milestones: Create Solution, Test Solution, Push Solution Mar 31, 2020
nizarmah added a commit that referenced this issue Apr 1, 2020
@nizarmah
Refactor: Updated the Code to Handle Multiple Directories inside Each…
970d180 
… Other with Correct Output Forwarding

The needed files to be minified are now found using *find* in bash. Output is *grepped* to avoid minified files. Then we get the output path, name, file extension of the output file, and run the npm script on the files.

The new approach fixes the bug represented in issue #1. In addition, there is a commit referenced in that issue to a failed workflow for that commit

This refactor fixes issue #1, makes the code cleaner, and more modular
nizarmah added a commit to nizarmah/auto-minify-test that referenced this issue Apr 1, 2020
@nizarmah
Updated Auto-Minify Directory to js/* to Handle Multiple Directories
61f21c4 
This tests the solution created for this [issue](nizarmah/auto-minify#1)
nizarmah added a commit that referenced this issue Apr 1, 2020
@nizarmah
Refactor: Updated the Code to Handle Multiple Directories inside Each…
0bebcdb 
… Other with Correct Output Forwarding

The needed files to be minified are now found using *find* in bash. Output is *grepped* to avoid minified files. Then we get the output path, name, file extension of the output file, and run the npm script on the files.

The new approach fixes the bug represented in issue #1. In addition, there is a commit referenced in that issue to a failed workflow for that commit

This refactor fixes issue #1, makes the code cleaner, and more modular
nizarmah added a commit to nizarmah/auto-minify-test that referenced this issue Apr 1, 2020
@nizarmah
Updated Auto-Minify to feature_multiple_dirs Branch to test Multiple …
e7d5955 
…Directories Feature

This tests the solution created for [handling multiple directories](nizarmah/auto-minify#1)
@nizarmah nizarmah mentioned this issue Apr 1, 2020
Merged
@nizarmah
Copy link
Owner

nizarmah commented Apr 1, 2020

Alright, so fixed it finally 🎉

Click here to go to the test branch where I tested it out. It finally works.

So, all you have to do now is just specify:

directory: 'js/*'

Make sure to update your action to v1.4

This works for both CSS and JS.

And it works just like find would in Linux.

Thanks a lot for your issue by the way ( :
And I'm glad that people are using the Action :D

Let me know if there's anything else you need, feel free to open a new issue : )

@nizarmah nizarmah closed this as completed Apr 1, 2020
@nizarmah nizarmah changed the title Handling multiple directories Handling nested directories Apr 1, 2020
@nizarmah nizarmah linked a pull request Apr 1, 2020 that will close this issue
Refactor: Updated the Code to Handle Multiple Directories inside Each… #2
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nizarmah nizarmah

Labels
bug Something isn't working enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Refactor: Updated the Code to Handle Multiple Directories inside Each… nizarmah/auto-minify
2 participants
@nizarmah @ekerstein