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

Liform Bundle #442

Closed
wants to merge 4 commits into from
Closed

Liform Bundle #442

wants to merge 4 commits into from

Conversation

GuiEloiSantos
Copy link

@GuiEloiSantos GuiEloiSantos commented Jul 31, 2018

Q A
License MIT

| ------------- | ---
| License       | MIT

Add Liform Bundle requirement
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request does not pass validation.

@GuiEloiSantos GuiEloiSantos changed the title master Liform Bundle Jul 31, 2018
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request does not pass validation.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request does not pass validation.

@GuiEloiSantos
Copy link
Author

GuiEloiSantos commented Jul 31, 2018

I think I'm facing the same scenario encountered at #163

@Nyholm
Copy link
Member

Nyholm commented Jul 31, 2018

Thank you.

I do not see that this is similar to #163. Your bundle follows the best practice file structure with means that Flex can install it without a recipe.

@GuiEloiSantos
Copy link
Author

@Nyholm you sure?

it composer has type: library

And even in it readme file is instructing the users to include it on app/AppKernel.php .

@Nyholm
Copy link
Member

Nyholm commented Aug 6, 2018

Im sure. Also, why does it have type: library? That should be changed since it clearly is a bundle, right?

@GuiEloiSantos
Copy link
Author

@Nyholm it should, I'm not a maintainer of this bundle though, and I thought I could use recipes so I don't have to include the bundle manually every time I install it.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request does not pass validation.

"env": {
"TEST_ENV_BOWLS": "test"
}
}
Copy link

Choose a reason for hiding this comment

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

Should end with a newline

"all"
]
},
"aliases": [
Copy link

Choose a reason for hiding this comment

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

Aliases not supported in the contrib repository

@@ -0,0 +1,13 @@
{
"bundles": {
Copy link

Choose a reason for hiding this comment

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

Indendation must be a multiple of 4 blanks

{
"bundles": {
"Limenius\\LiformBundle\\LimeniusLiformBundle": [
"all"
Copy link

Choose a reason for hiding this comment

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

Indendation must be a multiple of 4 blanks

"Limenius\\LiformBundle\\LimeniusLiformBundle": [
"all"
]
},
Copy link

Choose a reason for hiding this comment

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

Indendation must be a multiple of 4 blanks

"all"
]
},
"aliases": [
Copy link

Choose a reason for hiding this comment

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

Indendation must be a multiple of 4 blanks

},
"aliases": [
"liform-recipes"
],
Copy link

Choose a reason for hiding this comment

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

Indendation must be a multiple of 4 blanks

"aliases": [
"liform-recipes"
],
"env": {
Copy link

Choose a reason for hiding this comment

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

Indendation must be a multiple of 4 blanks

],
"env": {
"TEST_ENV_BOWLS": "test"
}
Copy link

Choose a reason for hiding this comment

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

Indendation must be a multiple of 4 blanks

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request does not pass validation.

"aliases": [
"liform-recipes"
]
}
Copy link

Choose a reason for hiding this comment

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

Should end with a newline

"all"
]
},
"aliases": [
Copy link

Choose a reason for hiding this comment

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

Aliases not supported in the contrib repository

Copy link
Member

Choose a reason for hiding this comment

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

Could you remove this please.

@maxhelias
Copy link
Contributor

@GuiEloiSantos you should submit a PR on the limenius/liform-bundle repository for switch the type "library" into "symfony-bundle". Like that the bundle will be compatible with Symfony

@maxhelias
Copy link
Contributor

And BTW, the section aliases is only for the official recipes

@Nyholm
Copy link
Member

Nyholm commented Nov 17, 2018

@symfony-flex-server review please

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Pull request does not pass validation.

"aliases": [
"liform-recipes"
]
}
Copy link

Choose a reason for hiding this comment

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

Should end with a newline

"all"
]
},
"aliases": [
Copy link

Choose a reason for hiding this comment

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

Aliases not supported in the contrib repository

Copy link
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

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

This recipe does not contain any information. Flex is smart enough to install this bundle without a recipe.

Feel free to reopen of you add non-default config to this recipe.

@@ -0,0 +1,10 @@
{
"bundles": {
"Limenius\\LiformBundle\\LimeniusLiformBundle": [
Copy link
Member

Choose a reason for hiding this comment

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

You could do this on one line

"all"
]
},
"aliases": [
Copy link
Member

Choose a reason for hiding this comment

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

Could you remove this please.

@Nyholm Nyholm closed this Nov 17, 2018
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.

3 participants