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

Include projects folder in the repo while ignoring sub folders #2842

Closed
wants to merge 1 commit into from

Conversation

gautamsi
Copy link
Member

improvement over #1832.

This adds projects folder in repo with readme and (git) ignores any other files/folders in projects folder. It is already included in workspace but not physically present

would be useful as many did not know it is there at their disposable and they have to use symlinked and other hacks to get it working. @Vultraz

Someone using this will need to make sure they are not polluting yarn.lock due to additional packages being used in those projects.

Don't think a changeset is needed but can add that if required.

@changeset-bot
Copy link

changeset-bot bot commented Apr 27, 2020

💥 No Changeset

Latest commit: adb4110

Merging this PR will not cause any packages to be released. If these changes should not cause updates to packages in this repo, this is fine 🙂

If these changes should be published to npm, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@Vultraz
Copy link
Contributor

Vultraz commented Apr 28, 2020

For the record, I've been directly working with the Todo demo since I've only been developing KS, not a project of my own, but this should be helpful once I get to actually doing that. 👍

And about the symlinks, BTW, I didn't set those up myself. KS handles that internally.

@MadeByMike
Copy link
Contributor

MadeByMike commented Apr 30, 2020

I added this to the main .gitignoremonths ago: demo-projects/test-project for this purpose.

I copied the todo and renamed it test-project, I'm a bit worried now that we are going to have multiple ways of doing this and #1832. It's really up to individual contributors where and how they want to develop and if we are going to offer something like this I say we offer one method.

I like it in the demo-project because it's automatically set-up as a workspace... but I don't have strong preferences.

If we think a readme is required, let's work out where the main project dev folder should be and add it there.

I'm less in favour of adding more folders to the root.

@gautamsi
Copy link
Member Author

gautamsi commented May 1, 2020

projects is already added in workspace config as part of earlier PR..

@MadeByMike
Copy link
Contributor

projects is already added in workspace config as part of earlier PR..

#1832 only adds the folder to .gitignore which doesn't actually create a folder. This PR actually adds a folder to the root of the workspace.

I'm not convinced that we should be signaling to contributors that "this is where we want you to develop projects in the mono-repo". AND if we do want to do that, I'd like to discuss if the demo-projects folder is better as it is already set-up with yarn workspaces and I think encourages people to think in terms of demo-projects, and encouraging the use the other demo-projects for development.

But that said, I'm not sure we should do either. It's really an individual preference that I don't think Keystone should have a strong opinion about.

@gautamsi gautamsi closed this May 5, 2020
@gautamsi
Copy link
Member Author

gautamsi commented May 5, 2020

I don't have any say either. anyone searching would know what are their option when they land here :)

@gautamsi gautamsi deleted the projects-folder-visible branch May 5, 2020 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants