Skip to content

remove index.html files#3788

Closed
drmmr763 wants to merge 8 commits intojoomla:stagingfrom
drmmr763:remove-index-files
Closed

remove index.html files#3788
drmmr763 wants to merge 8 commits intojoomla:stagingfrom
drmmr763:remove-index-files

Conversation

@drmmr763
Copy link
Copy Markdown
Contributor

Remove index.html files from the joomla-cms. Used from the CLI:
find . -name 'index.html' -type f -delete

@wilsonge
Copy link
Copy Markdown
Contributor

How do build scripts now work e.g. index references here https://github.com/joomla/joomla-cms/blob/staging/build/build.php#L86

Plus I guess we can remove this script here https://github.com/joomla/joomla-cms/blob/staging/build/indexmaker.php

@drmmr763
Copy link
Copy Markdown
Contributor Author

I'm wondering if we should provide a way for the user to generate these files in case their host doesn't support another way to prevent this sort of directory browsing. In which case - Ian's script might be useful there.

@mbabker
Copy link
Copy Markdown
Contributor

mbabker commented Jun 15, 2014

The build script "tags" files in each top level folder to ensure they're always in the packages. The index.html files were used because they were constant, and some of the top level folders don't have files within them, just more folders.

@wilsonge
Copy link
Copy Markdown
Contributor

wilsonge commented Oct 7, 2014

I hate to ask but are we gonna need to add each of these files into the script.php :(

@beat
Copy link
Copy Markdown
Contributor

beat commented Oct 7, 2014

btw, heads-up: git doesn't store at all completely empty folders, as it stores only files, so with this PR folders with only index.html will disappear ;-) .

@mbabker
Copy link
Copy Markdown
Contributor

mbabker commented Oct 7, 2014

I hate to ask but are we gonna need to add each of these files into the script.php :(

I wouldn't worry about it, if they're present on existing sites then so be it.

@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Oct 7, 2014

btw, heads-up: git doesn't store at all completely empty folders, as it stores only files, so with this PR folders with only index.html will disappear ;-) .

Examples (Override and Cache Folder in Backend and Frontend):
https://github.com/joomla/joomla-cms/tree/staging/administrator/cache
https://github.com/joomla/joomla-cms/tree/staging/administrator/language/overrides
https://github.com/joomla/joomla-cms/tree/staging/cache
https://github.com/joomla/joomla-cms/tree/staging/language/overrides

@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Oct 7, 2014

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.

I wouldn't remove this one, as it's an empty folder.

@infograf768
Copy link
Copy Markdown
Member

and also /logs

@Hackwar
Copy link
Copy Markdown
Member

Hackwar commented Oct 12, 2014

Could we make this simple? index.html stays in empty folders and everything else gets removed. I created a PR against @drmmr763 s repo to do that. Lets get this approved. 😄

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 is this in here? Can we keep this clean of other changes?

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.

hmm. apparently I was experimenting with something else at the time :P I'll update asap.

@drmmr763
Copy link
Copy Markdown
Contributor Author

Merged in @Hackwar's changes, fixed the silly com_config change that shouldn't have been there.

@nicksavov
Copy link
Copy Markdown
Contributor

@beat good catch! :)

@compojoom
Copy link
Copy Markdown
Contributor

Okay, just tested and all index.html are gone, except for cache/index.php and administrator/cache/index.php, but this is expected...

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/3788.

@wilsonge
Copy link
Copy Markdown
Contributor

umm shouldn't logs/index.html exist as well?

@Hackwar
Copy link
Copy Markdown
Member

Hackwar commented Oct 17, 2014

You are right...

@elkuku
Copy link
Copy Markdown
Contributor

elkuku commented Oct 17, 2014

What if we place a .gitignore file inside those empty folders (that might give the additional benefit of git-ignoring the contents)?

Oh, and a BIG 👍 for doing this 😉

@gwsdesk
Copy link
Copy Markdown

gwsdesk commented Nov 23, 2014

We should have a 'hands-off' policy on all 3rd party as discussed on the chat. So if we have those in Tiny do they have a valid function or can we do that different?

@Hackwar
Copy link
Copy Markdown
Member

Hackwar commented Nov 23, 2014

No, we should not have a hands-off policy for 3rd party libraries, because there is no reason to ship the complete documentation of phpmailer for example with Joomla. If people need that documentation, they can look at it at the phpmailer dev site. There is no reason for us to add a Megabyte of static docs to our repository from a third party. I would remove the index.html files in codemirror, too, because they, too, are simply documentation for a developer when they want to implement this feature in their system and thus unnecessary for a normal enduser. But I honestly couldn't care less about those files. leave them in if you want.

Regarding the TinyMCE index.html files: WE added those files into TinyMCE according to our rule at that time that all folders should have such an index.html. They are not originaly from TinyMCE, they are not necessary for the functioning of TinyMCE and they are completely useless otherwise as well.

@gwsdesk
Copy link
Copy Markdown

gwsdesk commented Nov 23, 2014

I partly agree but as Roland mentioned, they will be back in codemirror
or wherever as soon as an upgrade arrives. Understand me right please I
am all in favor to get rid of all those files as long as we do not break
things
On 11/23/2014 6:06 PM, Hannes Papenberg wrote:

No, we should not have a hands-off policy for 3rd party libraries,
because there is no reason to ship the complete documentation of
phpmailer for example with Joomla. If people need that documentation,
they can look at it at the phpmailer dev site. There is no reason for
us to add a Megabyte of static docs to our repository from a third
party. I would remove the index.html files in codemirror, too, because
they, too, are simply documentation for a developer when they want to
implement this feature in their system and thus unnecessary for a
normal enduser. But I honestly couldn't care less about those files.
leave them in if you want.

Regarding the TinyMCE index.html files: WE added those files into
TinyMCE according to our rule at that time that all folders should
have such an index.html. They are not originaly from TinyMCE, they are
not necessary for the functioning of TinyMCE and they are completely
useless otherwise as well.


Reply to this email directly or view it on GitHub
#3788 (comment).

@Hackwar
Copy link
Copy Markdown
Member

Hackwar commented Nov 23, 2014

As well as the documentation for phpmailer would be back if we copied it verbatim from the source. The job of the next guy updating codemirror would be to remove those again before commiting it to our repo. But since its simply useless bits and bytes that we are lugging around, its totally irrelevant if we have it or don't have it or if it gets added back in later on. Right now its identical to driving a rock around in your trunk. Doesn't hurt. Doesn't help either.

@gwsdesk
Copy link
Copy Markdown

gwsdesk commented Nov 23, 2014

It costs bandwidth so it does hurts as would in your example the
increase of weight takes more petrol so your remark is very valid "The
job of the next guy updating codemirror would be to remove those again
before committing it to our repo." So we could start by removing those
from this 3.4 release?
On 11/23/2014 6:18 PM, Hannes Papenberg wrote:

As well as the documentation for phpmailer would be back if we copied
it verbatim from the source. The job of the next guy updating
codemirror would be to remove those again before commiting it to our
repo. But since its simply useless bits and bytes that we are lugging
around, its totally irrelevant if we have it or don't have it or if it
gets added back in later on. Right now its identical to driving a rock
around in your trunk. Doesn't hurt. Doesn't help either.


Reply to this email directly or view it on GitHub
#3788 (comment).

@roland-d
Copy link
Copy Markdown
Contributor

Hannes does make a valid point, that the documentation can be looked up at the website of the 3rd party plugin. As long a removing documentation from a 3rd party plugin doesn't interfere with it's workings I am fine with it. I mean, if there is a help button inside the editor that breaks because we remove the index.html file, one can wonder if we should remove it.

This should indeed be the responsibility of the maintainer but I don't know if we have set maintainers or it is just someone seeing it needs an update and creates a PR. This person may not be aware of what has been done in the past. We should document this somewhere.

@Bakual
Copy link
Copy Markdown
Contributor

Bakual commented Nov 23, 2014

For the libraries we pull in using composer, we added the doc and testing files to our .gitignore. See lines starting at https://github.com/joomla/joomla-cms/blob/staging/.gitignore#L46
This way we can pull in the whole library repo using composer, and only what we really need gets pushed into our own repo.
But this gets offtopic here a bit 😄

@Bakual
Copy link
Copy Markdown
Contributor

Bakual commented Nov 23, 2014

I find this actually very hard to review since GitHub doesn't show me the whole diff. There are 1283 changed files, but it only shows me the first 300. Thus I can't review it online. I have to download the diff into my IDE and see what it shows. PhpStorm calculates that since about 20 minutes, so I doubt that's gonna work well 😄

I wonder how you guys reviewed/tested that 😄

@dgrammatiko
Copy link
Copy Markdown
Contributor

@Bakual Why don’t you apply this and just search in terminal for index.php? It sounds easier...

@Bakual
Copy link
Copy Markdown
Contributor

Bakual commented Nov 23, 2014

@DGT41 Don't tell me there is still a terminal in the year 2014. I prefer a GUI wherever I can use one.

@mbabker
Copy link
Copy Markdown
Contributor

mbabker commented Nov 23, 2014

GUI's are restrictive 😝

Command line interfaces expose the full toolset of whatever you're working with

/me steps off soapbox 😉

@Bakual
Copy link
Copy Markdown
Contributor

Bakual commented Nov 23, 2014

I used to think the same back in the days of DOS 😉

@dgrammatiko
Copy link
Copy Markdown
Contributor

DOS 6.22 Those were the days… 😄

@brianteeman
Copy link
Copy Markdown
Contributor

Pah -youngsters
On 23 Nov 2014 16:53, "Dimitris Grammatiko" notifications@github.com
wrote:

DOS 6.22 Those were the days… 😄


Reply to this email directly or view it on GitHub
#3788 (comment).

@infograf768
Copy link
Copy Markdown
Member

Did not we say that logs/index.html should stay?

@wilsonge
Copy link
Copy Markdown
Contributor

Yes

@okonomiyaki3000
Copy link
Copy Markdown
Contributor

I could be wrong but I reckon that 1283 is a few too many. By my count there should be 1239 deleted files. I arrived at this number by the following methodology so please let me know if it's wrong.

  • checkout a new branch based on staging
  • run the following shell commands to remove all index files then put them back in empty folders
find . -type f -name index.html | xargs rm
find . -type d -empty | xargs git checkout
  • commit the changes then run diff --name-status against staging and count the lines

@Hackwar
Copy link
Copy Markdown
Member

Hackwar commented Dec 10, 2014

Since this PR is unfortunately not behaving like a good cheese and thus gets better with ageing, I'm thinking if it would be better to do this in batches instead. Like, removing all index.html from /components in one PR and then all from /modules in another. In the hopes that it can be reviewed by a CMS maintainer more easily...

@Bakual
Copy link
Copy Markdown
Contributor

Bakual commented Dec 10, 2014

I fully agree with Hannes. This PR can't easily reviewed due to the sheer amount of files.
GitHub only shows the first 300 changed files.

If someone wants to do smaller PRs with max 300 removed files, then I can review and merge them fast.

Make sure you don't remove index.html files from empty folders as that would remove the folder in Git as well.

@okonomiyaki3000
Copy link
Copy Markdown
Contributor

I can do it soon.

@okonomiyaki3000
Copy link
Copy Markdown
Contributor

here you go: #5377 #5378 #5378 #5379 #5380 #5381 #5382

@Hackwar
Copy link
Copy Markdown
Member

Hackwar commented Dec 10, 2014

@okonomiyaki3000 THANK YOU! 😄

@okonomiyaki3000
Copy link
Copy Markdown
Contributor

@Hackwar No problem. It's really easy to do something like this if you use the command line, right @mbabker ?

@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Dec 10, 2014

@Bakual here is the PR to remove also the indexmaker script: #5383

@Bakual
Copy link
Copy Markdown
Contributor

Bakual commented Dec 10, 2014

Going to close this PR in favor of the smaller ones.
Thanks all!

@Bakual Bakual closed this Dec 10, 2014
@HLeithner HLeithner mentioned this pull request Feb 28, 2021
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.