Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Migrate to LuckyTemplate - Part 4 of ... #810

Merged
merged 1 commit into from
Jun 25, 2023
Merged

Conversation

mdwagner
Copy link
Contributor

  • Changes browser_authentication_app_skeleton to use lucky_template

@mdwagner mdwagner requested a review from jwoertink June 25, 2023 14:31
@mdwagner mdwagner self-assigned this Jun 25, 2023
end
end
actions_dir.add_folder("mixins") do |mixins_dir|
mixins_dir.add_file(".keep")
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can knock out #778 too while you're in here? This issue is basically saying no .keep inside folders that will always have files generated. We can also come back to that later once everything is in place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only issue I have removing .keep files are that it removes the intention of whether a particular folder structure should always exist, e.g. mixins is a folder name you expect to see in a project. Or support is a folder name you expect to see in spec. That's my opinion, otherwise I agree you don't need to include them (although, they don't cause any harm in having them).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could always add an option to the CLI that doesn't add those files, which seems like the better option here.

Copy link
Member

Choose a reason for hiding this comment

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

I'm cool with that too. The files being there don't really bother me. We can just leave it for now and worry about that in a future PR.

end

private def emails_folder
LuckyTemplate.create_folder do |emails_dir|
Copy link
Member

Choose a reason for hiding this comment

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

That's interesting that this already knows what directory its in. Pretty neat!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not that it already knows, it's more that I'm making it clear that I expect it to be this way. You still need to add it to Folder#insert_folder and provide a name (which could be something other than "emails"). This is just so you can separate out the folders for better code organization or for reuse.

@mdwagner mdwagner merged commit 9de394d into main Jun 25, 2023
@mdwagner mdwagner deleted the feat/lucky-template-3 branch June 25, 2023 22:33
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants