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

don't redirect to "downloader" (which doesn't exists) #1523

Merged
merged 1 commit into from
May 28, 2021
Merged

don't redirect to "downloader" (which doesn't exists) #1523

merged 1 commit into from
May 28, 2021

Conversation

fballiano
Copy link
Contributor

In the main index.php there's a check for the app/Mage.php (which is 99.9% useless) and a redirect to the "magento downloader".

Downloader was removed with #952 so it's useless to redirect to the downloader subdir. Also, I think the purpose of checkin if the MAGENTO_ROOT . '/app/Mage.php' is useless, one file check for every request while magento wouldn't work at all if it didn't exist, I think that check has to be removed, there's no point in executing it.

Downloader was removed with #952 so it's useless to redirect to the downloader subdir. Also, I think the purpose of checkin if the MAGENTO_ROOT . '/app/Mage.php' is useless, one file check for every request while magento wouldn't work at all if it didn't exist, I think that check has to be removed, there's no point in executing it.
index.php Show resolved Hide resolved
Copy link
Contributor

@kiatng kiatng left a comment

Choose a reason for hiding this comment

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

I agree that the check is not useful. For new install, it may be useful to inform that file is missing, but PHP would throw an error anyway. For development or production, it's totally useless.

@fballiano
Copy link
Contributor Author

I think this should really be merged, the downloader doesn't exist since some time and this check is just a waste of CPU clocks, let's close this and move on to other stuff no? ;-)

I know I'm a pain in the *** but I keep checking for my issues/PR and so I don't use the time to do other stuff for the project.

@fballiano
Copy link
Contributor Author

what should we do about this?

@fballiano
Copy link
Contributor Author

@Flyingmana @sreichel @kiatng I think this one could be easily merged, otherwise close it and let's forget it (but I can't see why)

@kiatng
Copy link
Contributor

kiatng commented May 5, 2021

I agree that this should be merged to v20.

@Flyingmana Flyingmana merged commit 87c03d7 into OpenMage:20.0 May 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.

5 participants