Pulling latest commits

[ctasca]

No description provided.

[alistairstead]

Hi Carlo,

We keep debating this convention and strict validation. The current thought was that by using and enforcing lowercase then this reflects the string you would pass to the Mage::getModel and other factory methods when following Magento best practices.

The vision for the MageTest tools is to coach best practice as well as support development.

However if you think this should be re-thought please could you maybe add some comments with example use cases?

Regards

[debo]

@alistairstead we where discussing this with @drewHunter the other day and I think we might need to rethink this.
Now the main point to consider is: "Do we want to be strict and say, legacy code shouldn't be specced?"
If so than we might want to leave the current implementation as it is, however if we instead want to allow retroactivity we definitely have to support a more loose validation. Drew for example is trying to do so on an existing project clearly struggling and being unable to proceed easily.

As usual I'm open to any constructive suggestions.

[alistairstead]

Does this prevent specing legacy code? Surly this just prevents the tool from generating the spec.. 11 lines of code.

Maybe we could relax the validation but print out a warning after the command that coaches best practice of using lowercase and why?

[debo]

Yes it does because in an ideal world you don't want to create a camel cased module/classes sets because as you said we would like to follow Magento best practises, but sometimes we inherit code and we have to deal with it and only in that case it will fail as it is right now.
If we are speccing from scratch I don't see why we don't want to follow Magento standards if that make sense, obvioulsy, being perfectly aware that Magento itself fails at it sometime we should indeed relax the validation.

[alistairstead]

I'm still +1 for strict validation within the commands.

If you need to spec legacy code you can create the specs manually.

That way we reflect the identifiers used internally for the classes. However with Magento 2 this is changing...

However this is an open project so other views are welcome.

[ctasca]

Hi Alistair,

I have thought this through a lot myself before sending a pull request, but
it is as a result of trying to use MageBehat and MageSpec with existing
projects.
For example a module named PayLater_PayLater I needed to spec, always
failed. I do understand that for models, blocks, and helpers names should
not be camel-cased, but doing it for Company and Module name is pretty
common practice in Magento.

I firstly discovered this when setting it up on my local and try to spec
the MageTool module. I couldn't because Autoloader was looking for Magetool.

Magento themselves do it with Mage_CurrencySymbol, Mage_PageCache,
Mage_ImportExport, etc...and I can give you a long list of third-party
modules camel-casing their namespace.

Magento should live on a case-sensitive environment anyway, so I don't
think allowing for this would be going against Magento best practices. IMHO
only Model, Block and Helper classes should have camel-cased names.

By keeping it this way, it means nobody with such module namespaces can
start creating specs. I have been using my updated version for a while now
and works without problems, but would be nice to put in composer dependency
to the real one ;)

Thanks

Kind Regards
Carlo

On Wed, Oct 9, 2013 at 10:04 PM, Alistair Stead notifications@github.comwrote:

Hi Carlo,

We keep debating this convention and strict validation. The current
thought was that by using and enforcing lowercase then this reflects the
string you would pass to the Mage::getModel and other factory methods when
following Magento best practices.

The vision for the MageTest tools is to coach best practice as well as
support development.

However if you think this should be re-thought please could you maybe add
some comments with example use cases?

Regards

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/41#issuecomment-26008582
.

Kind Regards

Carlo Tasca**

[alistairstead]

I very well made case and I think you are right!

Will merge ASAP

Sent from my iPhone

On 18 Oct 2013, at 21:34, ctasca notifications@github.com wrote:

Hi Alistair,

I have thought this through a lot myself before sending a pull request, but
it is as a result of trying to use MageBehat and MageSpec with existing
projects.
For example a module named PayLater_PayLater I needed to spec, always
failed. I do understand that for models, blocks, and helpers names should
not be camel-cased, but doing it for Company and Module name is pretty
common practice in Magento.

I firstly discovered this when setting it up on my local and try to spec
the MageTool module. I couldn't because Autoloader was looking for Magetool.

Magento themselves do it with Mage_CurrencySymbol, Mage_PageCache,
Mage_ImportExport, etc...and I can give you a long list of third-party
modules camel-casing their namespace.

Magento should live on a case-sensitive environment anyway, so I don't
think allowing for this would be going against Magento best practices. IMHO
only Model, Block and Helper classes should have camel-cased names.

By keeping it this way, it means nobody with such module namespaces can
start creating specs. I have been using my updated version for a while now
and works without problems, but would be nice to put in composer dependency
to the real one ;)

Thanks

Kind Regards
Carlo

On Wed, Oct 9, 2013 at 10:04 PM, Alistair Stead notifications@github.comwrote:

Hi Carlo,

We keep debating this convention and strict validation. The current
thought was that by using and enforcing lowercase then this reflects the
string you would pass to the Mage::getModel and other factory methods when
following Magento best practices.

The vision for the MageTest tools is to coach best practice as well as
support development.

However if you think this should be re-thought please could you maybe add
some comments with example use cases?

Regards

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/41#issuecomment-26008582
.

Kind Regards

Carlo Tasca**
—
Reply to this email directly or view it on GitHub.

[ctasca]

Thanks a million :)

On Fri, Oct 18, 2013 at 9:37 PM, Alistair Stead notifications@github.comwrote:

I very well made case and I think you are right!

Will merge ASAP

Sent from my iPhone

On 18 Oct 2013, at 21:34, ctasca notifications@github.com wrote:

Hi Alistair,

I have thought this through a lot myself before sending a pull request,
but
it is as a result of trying to use MageBehat and MageSpec with existing
projects.
For example a module named PayLater_PayLater I needed to spec, always
failed. I do understand that for models, blocks, and helpers names
should
not be camel-cased, but doing it for Company and Module name is pretty
common practice in Magento.

I firstly discovered this when setting it up on my local and try to spec
the MageTool module. I couldn't because Autoloader was looking for
Magetool.

Magento themselves do it with Mage_CurrencySymbol, Mage_PageCache,
Mage_ImportExport, etc...and I can give you a long list of third-party
modules camel-casing their namespace.

Magento should live on a case-sensitive environment anyway, so I don't
think allowing for this would be going against Magento best practices.
IMHO
only Model, Block and Helper classes should have camel-cased names.

By keeping it this way, it means nobody with such module namespaces can
start creating specs. I have been using my updated version for a while
now
and works without problems, but would be nice to put in composer
dependency
to the real one ;)

Thanks

Kind Regards
Carlo

On Wed, Oct 9, 2013 at 10:04 PM, Alistair Stead <
notifications@github.com>wrote:

Hi Carlo,

We keep debating this convention and strict validation. The current
thought was that by using and enforcing lowercase then this reflects
the
string you would pass to the Mage::getModel and other factory methods
when
following Magento best practices.

The vision for the MageTest tools is to coach best practice as well as
support development.

However if you think this should be re-thought please could you maybe
add
some comments with example use cases?

Regards

—
Reply to this email directly or view it on GitHub<
https://github.com/MageTest/MageSpec/pull/41#issuecomment-26008582>
.

Kind Regards

Carlo Tasca**
—
Reply to this email directly or view it on GitHub.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/41#issuecomment-26627800
.

Kind Regards

Carlo Tasca**

[alistairstead]

[asarturas]

Hi,

Sorry for bringing it up after merge, but this is not correct.
The recommended magento convention is to do it in lowercase. We can allow uppercase for these special cases with legacy code, but we have to suggest using lowercase in our error messages.

Arturas

[asarturas]

See my pull request #46, which reverts changes in messages