Skip to content

Cleaning up JArchiveBzip2#6495

Merged
Kubik-Rubik merged 3 commits intojoomla:stagingfrom
Mathewlenning:CLEANING-JArchiveBzip2
May 8, 2016
Merged

Cleaning up JArchiveBzip2#6495
Kubik-Rubik merged 3 commits intojoomla:stagingfrom
Mathewlenning:CLEANING-JArchiveBzip2

Conversation

@Mathewlenning
Copy link
Copy Markdown
Contributor

Isolated JError into temporary method and separated old extraction logic from streaming logic.

I'm not sure where this is used, but most likely in the installer.
I did a find usage with PHPStorm, but the only results I got where from the unit tests.

So if anyone knows where it is used could you let me know.

…separated old extraction logic from streaming logic.
@Mathewlenning
Copy link
Copy Markdown
Contributor Author

For some reason this didn't get tagged and TCI wasn't initiated. Is Bzip2 on the exclude list?

@wilsonge
Copy link
Copy Markdown
Contributor

Nothing should be on the exclude list. I'll try and work out what's going on!

@Mathewlenning
Copy link
Copy Markdown
Contributor Author

It started up after I pushed a few style fixes. I'm guessing it was just a hiccup in TCI

@Kubik-Rubik
Copy link
Copy Markdown
Member

Looks good to me. We won't add this in the 3.4.1 but after another successful test we can merge it for the 3.4.2 version. Thank you @Mathewlenning!

@Kubik-Rubik Kubik-Rubik added this to the Joomla! 3.4.2 milestone Mar 19, 2015
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do you unset this here? Shouldn't this be set to null or something?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That is the original behavior. Changing an established side effect without a good reason doesn't make sense. Setting it to null has no benefit to cleanliness or clarity, but it does present the possibility of breaking something if someone was expecting it to be unset.

@wilsonge wilsonge removed this from the Joomla! 3.5.0 milestone Nov 6, 2015
@Kubik-Rubik Kubik-Rubik added this to the Joomla 3.6.0 milestone May 8, 2016
@Kubik-Rubik
Copy link
Copy Markdown
Member

Thank you @Mathewlenning!

@Kubik-Rubik Kubik-Rubik merged commit 578799b into joomla:staging May 8, 2016
roland-d added a commit to roland-d/joomla-cms that referenced this pull request May 8, 2016
…leanup-installer-plugins

* 'staging' of https://github.com/joomla/joomla-cms:
  Smart Search: utf8_strpos: Offset must be an integer (joomla#10303)
  Removed an redundant else clause from JFeedParser::processElement (joomla#7961)
  CLEANING-JDataSet (joomla#7947)
  CLEANING-JAdapter (joomla#6679)
  Same  treatment as JArchiveBzip2 and JArchiveGzip (joomla#6515)
  Cleaning up JArchiveBzip2 (joomla#6495)
  Removed the nested conditional switch construct (joomla#6338)
  add edit tooltip to modules (joomla#10295)
  New installer plugins: remove the dot in the plugin name and other language review (joomla#10299)
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.

5 participants