Skip to content
This repository was archived by the owner on Nov 26, 2017. It is now read-only.

Allow template and library extension types to use scriptfiles#702

Closed
marcel-kanter wants to merge 8 commits intojoomla:stagingfrom
marcel-kanter:installer
Closed

Allow template and library extension types to use scriptfiles#702
marcel-kanter wants to merge 8 commits intojoomla:stagingfrom
marcel-kanter:installer

Conversation

@marcel-kanter
Copy link
Copy Markdown

This pull request makes changes to the template and library extension installer to allow for a scriptfile to be included on a template or a library.

This will allow them to have a preflight method, for example to check for any dependencies, such as another extension's presence or a minimum Joomla! version.

The scripts stay together with the rest of the files in extension_root with the other files.

@marcel-kanter
Copy link
Copy Markdown
Author

Shall we fix the use of '/' in the files and replace them with DS?

@joomla-jenkins
Copy link
Copy Markdown

Unit testing complete. There were 0 failures and 0 errors from 1971 tests and 11145 assertions.
Checkstyle analysis reported 199 warnings and 62 errors.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should be the current platform version being developed towards, currently 11.4

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

12.1 I guess.

@marcel-kanter
Copy link
Copy Markdown
Author

I'll fix the style issues.
Shall we fix the use of '/' in the files and replace them with DS?

@joomla-jenkins
Copy link
Copy Markdown

Build triggered by changes to the head.

Unit testing complete. There were 0 failures and 0 errors from 1971 tests and 11146 assertions.
Checkstyle analysis reported 199 warnings and 51 errors.

@mbabker
Copy link
Copy Markdown
Contributor

mbabker commented Jan 4, 2012

We're leaning more towards / versus DS since PHP is able to handle both slashes as separators just fine, including mixed use.

@mbabker
Copy link
Copy Markdown
Contributor

mbabker commented Jan 4, 2012

Not "official", but has a list of most of your code style errors as of yesterday - http://elkuku.github.com/pulltester/pulls/702

@marcel-kanter
Copy link
Copy Markdown
Author

Is it possible to trigger the style check manually? Since i've corrected the style errors in the last 2 commits and they are still reported? Just take the if in line 125 as an example. there is no missing space.
Used your checker already ;)

@mbabker
Copy link
Copy Markdown
Contributor

mbabker commented Jan 4, 2012

If you have phpcs installed, yes. There's some info on developer.joomla.org to help you.

-Michael

Please pardon any errors, this message was sent from my iPhone.

On Jan 4, 2012, at 1:30 PM, marcel-kanter reply@reply.github.com wrote:

Is it possible to trigger the style check manually? Since i've corrected the style errors in the last 2 commits and they are still reported? Just take the if in line 125 as an example. there is no missing space.


Reply to this email directly or view it on GitHub:
#702 (comment)

@mbabker
Copy link
Copy Markdown
Contributor

mbabker commented Jan 4, 2012

I sent you a pull request updating the branch to the current master commit as well as fixing the code style errors.

@marcel-kanter
Copy link
Copy Markdown
Author

Pulled It. Tnx

@joomla-jenkins
Copy link
Copy Markdown

Build triggered by changes to the head.

Unit testing complete. There were 0 failures and 0 errors from 1971 tests and 11146 assertions.
Checkstyle analysis reported 199 warnings and 0 errors.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what is the reason to change this behaviour?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Install and Update are the same with a little but relevant difference: Install should run when it is not installed and update should run when there is already data oder files present.

  1. This is what a creator of an library would expect: Install does install and update is update and NOT re-install.
  2. The old code always removed the files before an re-install to do an update (which could be incremental) or there are maybe files that the install-script generated dynamicly and the update may need these files. Some inis for example...
  3. Since the creator of the extension can do anything in the script, we should not asume that the library is only files anymore. So we should give the control to the creator of the library. Btw. I'm already thinking about where to store configuration of my new library.
  4. The installers for the other extensions are correct already. So why keep this incorrect behaviour here?

@realityking
Copy link
Copy Markdown
Contributor

The pull is not in a mergeable state.

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.

6 participants