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

Simplified application directory structure #19

Closed
leggetter opened this Issue Oct 4, 2013 · 9 comments

Comments

Projects
None yet
2 participants
@leggetter
Contributor

leggetter commented Oct 4, 2013

Let's use this space to keep track of the possible options - and final decision. Please add any additional ideas structure ideas here (after discussion, if required).

And use comments to discuss (or a meeting if required).

1. No Default Aspect directory & -blade directories

This is probably more aligned with current simple application structures. The Aspect content is shifted into the root of the app. Blades are in directories in the root of the app with the directories having a suffix of -blade (similar to the -aspect and -bladeset suffixes).

+- myapp
|   \- <bladename>-blade
|   \- <bladename>-blade
|   \- <bladename>-blade
|   \- libs
|   \- src (JS source for Default Aspect)
|   \- themes (themes for Default Aspect)
|   -- app.conf
|   -- index.html (Default Aspect seed file)

2. Default Aspect directory & -blade directories

+- myapp
|   \- <bladename>-blade
|   \- <bladename>-blade
|   \- <bladename>-blade
|   \- default-aspect
|   \- libs
|   -- app.conf

3. No Default Aspect directory & blades directory for all Blades

In this example the blades go underneath a blades directory. This is consistent with blades that reside within BladeSets.

+- myapp
|   \- blades
|       \- blade1
|       \- blade2
|       \- blade3
|   \- libs
|   \- resources (resources for Default Aspect)
|   \- src (JS source for Default Aspect)
|   \- themes (themes for Default Aspect)
|   -- app.conf
|   -- index.html (Default Aspect seed file)

4. Default Aspect directory & blades directory for all Blades

In this example the blades go underneath a blades directory. This is consistent with blades that reside within BladeSets.

We keep default-aspect.

+- myapp
|   \- blades
|       \- blade1
|       \- blade2
|       \- blade3
|   \- default-aspect
|   \- libs
|   -- app.conf

Blades Simplified Directory Structure

This is actually issue #80.

The main suggested change here is to .js files directly under the src directory to use the blade require prefix (remove the need to repeat the app directory structure).

+- myblade
|   \- resources
|       \- html
|       \- 18n
|       \- xml
|       -- aliases.xml
|   \- src
|       -- MyBlade.js
|       -- MyClass.js
|   \- tests
|   \- themes
|   \- workbench

Note: I've left out #880 (simplify tests directory structure) from this.

@andyberry88 andyberry88 added 0 - Backlog and removed 0 - Backlog labels Apr 24, 2014

@leggetter leggetter referenced this issue Jul 30, 2014

Closed

Issues to be fixed before next workshop #873

2 of 6 tasks complete

@andyberry88 andyberry88 added this to the 0.12 milestone Aug 1, 2014

@andyberry88 andyberry88 added the size 2 label Aug 1, 2014

@leggetter

This comment has been minimized.

Show comment
Hide comment
@leggetter

leggetter Aug 4, 2014

Contributor

My vote would probably go with 2 or 4. This means we need to keep the idea of Aspects as a core concept. However, I think it results in the directory structure being clean and related assets being grouped together under a clearly marked directory.

I also think that having src at the top level may result in some confusion with code in Blades trying to reference it as shared code.

Contributor

leggetter commented Aug 4, 2014

My vote would probably go with 2 or 4. This means we need to keep the idea of Aspects as a core concept. However, I think it results in the directory structure being clean and related assets being grouped together under a clearly marked directory.

I also think that having src at the top level may result in some confusion with code in Blades trying to reference it as shared code.

@andyberry88 andyberry88 added feature and removed size 2 labels Aug 4, 2014

@andyberry88

This comment has been minimized.

Show comment
Hide comment
@andyberry88

andyberry88 Aug 5, 2014

Member

My vote is for 4. As you said above This is consistent with blades that reside within BladeSets.

  • I assume the require path for a blade that doesn't have a bladeset becomes appns/bladename?
  • What happens when new bladesets are created? Can 'simplified' blades live beside bladesets? (I can't see any reason why not but worth checking)
  • How can I use the CLI to create a new blade without a bladeset? We've previously discussed refering to these blades as having a default bladeset so ./brjs create-blade myApp default myBlade would create a bladesetless blade. This would probably help ensure the current commands/APIs work since every app would have an implicit 'default' bladeset.
Member

andyberry88 commented Aug 5, 2014

My vote is for 4. As you said above This is consistent with blades that reside within BladeSets.

  • I assume the require path for a blade that doesn't have a bladeset becomes appns/bladename?
  • What happens when new bladesets are created? Can 'simplified' blades live beside bladesets? (I can't see any reason why not but worth checking)
  • How can I use the CLI to create a new blade without a bladeset? We've previously discussed refering to these blades as having a default bladeset so ./brjs create-blade myApp default myBlade would create a bladesetless blade. This would probably help ensure the current commands/APIs work since every app would have an implicit 'default' bladeset.
@leggetter

This comment has been minimized.

Show comment
Hide comment
@leggetter

leggetter Aug 5, 2014

Contributor

I assume the require path for a blade that doesn't have a bladeset becomes appns/bladename

👍

What happens when new bladesets are created? Can 'simplified' blades live beside bladesets? (I can't see any reason why not but worth checking)

Yes. I believe they should be able to live alongside.

How can I use the CLI to create a new blade without a bladeset? We've previously discussed refering to these blades as having a default bladeset so ./brjs create-blade myApp default myBlade would create a bladesetless blade. This would probably help ensure the current commands/APIs work since every app would have an implicit 'default' bladeset.

I'm not too keen on the need to include default when nothing actually exists with that name. It's an internal name that I think we should hide.

Would it be possible to update the create-blade to make the bladeset parameter optional? e.g. if two parameters are used then the command creates a blade.

create-blade myapp bladename

If three parameters are passed then it creates the blade within the bladeset.

Contributor

leggetter commented Aug 5, 2014

I assume the require path for a blade that doesn't have a bladeset becomes appns/bladename

👍

What happens when new bladesets are created? Can 'simplified' blades live beside bladesets? (I can't see any reason why not but worth checking)

Yes. I believe they should be able to live alongside.

How can I use the CLI to create a new blade without a bladeset? We've previously discussed refering to these blades as having a default bladeset so ./brjs create-blade myApp default myBlade would create a bladesetless blade. This would probably help ensure the current commands/APIs work since every app would have an implicit 'default' bladeset.

I'm not too keen on the need to include default when nothing actually exists with that name. It's an internal name that I think we should hide.

Would it be possible to update the create-blade to make the bladeset parameter optional? e.g. if two parameters are used then the command creates a blade.

create-blade myapp bladename

If three parameters are passed then it creates the blade within the bladeset.

@andyberry88

This comment has been minimized.

Show comment
Hide comment
@andyberry88

andyberry88 Aug 5, 2014

Member

Would it be possible to update the create-blade to make the bladeset parameter optional? e.g. if two parameters are used then the command creates a blade.

We can do that but every command would now need to work in 2 different ways and the fact that bladesets are optional is no longer abstracted away. This would also be the same with the API and possibly plugins that now need to understand this new optional structure.

Member

andyberry88 commented Aug 5, 2014

Would it be possible to update the create-blade to make the bladeset parameter optional? e.g. if two parameters are used then the command creates a blade.

We can do that but every command would now need to work in 2 different ways and the fact that bladesets are optional is no longer abstracted away. This would also be the same with the API and possibly plugins that now need to understand this new optional structure.

@leggetter

This comment has been minimized.

Show comment
Hide comment
@leggetter

leggetter Aug 5, 2014

Contributor

the fact that bladesets are optional is no longer abstracted away

Not sure what you mean by this.

The default becomes creating blades that do not reside within bladesets. Bladesets are more advanced feature and - in terms of the user journey - are only exposed when there becomes a need for it ("If you don't want to move shared assets into a library and you want to share them between related blades then you can use bladesets").

Contributor

leggetter commented Aug 5, 2014

the fact that bladesets are optional is no longer abstracted away

Not sure what you mean by this.

The default becomes creating blades that do not reside within bladesets. Bladesets are more advanced feature and - in terms of the user journey - are only exposed when there becomes a need for it ("If you don't want to move shared assets into a library and you want to share them between related blades then you can use bladesets").

@andyberry88

This comment has been minimized.

Show comment
Hide comment
@andyberry88

andyberry88 Aug 5, 2014

Member

OK, I'd assumed they would be optional rather than entirely hidden away. Completely hiding bladesets does make sense so commands should (by default) not require the bladeset name.

We need to work out whether we want the API to directly reflect the structure or still have the concept of a default bladeset (which may get confusing). I'll start looking at making bladesets optional and see what issues I come across.

Member

andyberry88 commented Aug 5, 2014

OK, I'd assumed they would be optional rather than entirely hidden away. Completely hiding bladesets does make sense so commands should (by default) not require the bladeset name.

We need to work out whether we want the API to directly reflect the structure or still have the concept of a default bladeset (which may get confusing). I'll start looking at making bladesets optional and see what issues I come across.

@andyberry88

This comment has been minimized.

Show comment
Hide comment
@andyberry88

andyberry88 Aug 13, 2014

Member

@leggetter have you had an ideas on tidying up the 'tests' directory structure (#880)? From what I can see there isn't a huge amount of simplification to do after src and test package directories become optional.

Member

andyberry88 commented Aug 13, 2014

@leggetter have you had an ideas on tidying up the 'tests' directory structure (#880)? From what I can see there isn't a huge amount of simplification to do after src and test package directories become optional.

@leggetter

This comment has been minimized.

Show comment
Hide comment
@leggetter

leggetter Aug 21, 2014

Contributor

@andyberry88 in the latest develop branch I still see the app name duplicated underneath src:

.
├── index.html
├── resources
├── src
│   └── brjstodo
│       └── App.js

Is this still a work in progress?

Contributor

leggetter commented Aug 21, 2014

@andyberry88 in the latest develop branch I still see the app name duplicated underneath src:

.
├── index.html
├── resources
├── src
│   └── brjstodo
│       └── App.js

Is this still a work in progress?

@andyberry88

This comment has been minimized.

Show comment
Hide comment
@andyberry88

andyberry88 Aug 27, 2014

Member

Is this still a work in progress?

@leggetter it was. It should now be fixed.

Member

andyberry88 commented Aug 27, 2014

Is this still a work in progress?

@leggetter it was. It should now be fixed.

@andyberry88 andyberry88 removed the 5 - Done label Sep 4, 2014

dchambers added a commit that referenced this issue Dec 8, 2014

Squashed 'brjs-sdk/workspace/sdk/libs/javascript/topiarist/' changes …
…from e2f0020..b8b2df2

b8b2df2 Merge pull request #19 from bit-shifter/ignore-static-properties-when-verifying-implement
607a0db prevents static properties (i.e. not functions) from causing #implements to fail

git-subtree-dir: brjs-sdk/workspace/sdk/libs/javascript/topiarist
git-subtree-split: b8b2df23b3b5340a03e0b9297a1044748ceb2fb9

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment