Skip to content

4506 remove app hardcode #4610

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

Closed
wants to merge 2 commits into from
Closed

Conversation

dopry
Copy link

@dopry dopry commented Feb 11, 2017

This PR removes the hard coded 'app' in the src folder. You can now merrily call the folder with the app under src whatever you want.

I ran the existing generate tests to get an idea of possible values for currentDir.

If you want to test manually to see the original behavior is preserved in commit 1daa11a
and commit daaad6c maintains the original behavior with app, but also allows for other subfolder names.

closes #4506

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@dopry dopry force-pushed the 4506_remove_app_hardcode branch from daaad6c to e47ac84 Compare February 11, 2017 01:56
@googlebot
Copy link

CLAs look good, thanks!

@dopry
Copy link
Author

dopry commented Feb 11, 2017

@Brocco, does this work for a PR?

@Brocco
Copy link
Contributor

Brocco commented Feb 12, 2017

This is a good start, but to be clear is this PR to just make it possible to change the dir name manually?

If so, this will work, but please add some documentation here: https://github.com/angular/angular-cli/tree/master/docs/documentation/stories

If not, then you will also need to update the ng2 blueprint (used for ng new) to allow for this change upon creating a new app.

@dopry
Copy link
Author

dopry commented Feb 13, 2017

Yes. I only want to enable changing the path manually. I don't want to encourage people to deviate from convention, just enable legacy projects to use the generate features. If their is demand for a CLI option, I'd say add it in later. I'll add a user story in the next day or so when I get time to work on this again.

.

@Brocco
Copy link
Contributor

Brocco commented Feb 15, 2017

There are other parts of the CLI that are still dependent upon an app folder named app. For example here

@dopry
Copy link
Author

dopry commented Feb 15, 2017

@Brocco, I see that. So here are some questions. What should be used in place of app? Should this be app.prefix as mentioned in @angular/cli/lib/config/schema.json? or should we add a new property to apps? or should app.root include the complete path to the root module of the application?

we can modify the whole generation process to be ignorant of path.. and just work relative to the first angular.module found by recursing parent directories from CWD. Or we can use this more explicit property from the config. The former is probably more resilient in terms of generation working with both ngcli built and non-ngcli built projects. The latter means we need a more clear understanding of how these properties are intended to be used.

@Brocco
Copy link
Contributor

Brocco commented Feb 16, 2017

I would be in favor of adding a separate config file entry to store the name of the directory. We can't use the prefix property because it may or may not match the value.

Also, please review the contributing doc to see how to format your commit message.

@dopry
Copy link
Author

dopry commented Feb 16, 2017 via email

@Brocco
Copy link
Contributor

Brocco commented Feb 16, 2017

The prefix is used to prepend selectors when you generate components and directives.

@filipesilva
Copy link
Contributor

Superseded by #4754, which does the same but at a larger scale.

@dopry
Copy link
Author

dopry commented Feb 22, 2017

@felipesilva #4754 doesn't remove the hard coded string app from the path. Look at the changes in dynamic path parser... It does not do the same thing as this PR.

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

I don't want to call my 'app', 'app'.
4 participants