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

Separate profiler view to a template file #4653

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

ashiina
Copy link

@ashiina ashiina commented May 29, 2016

A PR for this issue: #4477

This PR simply removes the HTML from libraries/Profiler.php , and instead reads from a separate HTML template file application/views/profiler/profiler_template.php .
Developers will be free to modify the template to suit their needs.

@narfbg
Copy link
Contributor

narfbg commented May 30, 2016

Haven't looked into it yet, just referencing #3847 as relevant.

@ashiina
Copy link
Author

ashiina commented May 30, 2016

@narfbg Thanks for the reference; didn't know there was a similar PR before. Since my current commit requires a newly implemented view, it will apply to your past comment:

If 3.0.0 requires nothing for this to work, and then you change it in 3.0.1 to require a view, how is it not a BC break?

To keep BC I cannot introduce any new files, so frankly I will propose just sticking the content of application/views/profiler/profiler_template.php into my newly refactored Profiler.php as a separate method or something.
Not pretty, I admit, but much better than the current state of Profiler.php.

Please let me know if I am heading the wrong direction.

@narfbg
Copy link
Contributor

narfbg commented May 31, 2016

This is targeting 3.1.0, where we'll have a few minor BC breaks like this, so ... no, you don't need to do that.

@ashiina
Copy link
Author

ashiina commented May 31, 2016

@narfbg Great, thanks! I personally prefer a separate view for the profiler (even by breaking BC) so that's good news.
Please let me know if there are any poor code or design decisions which require fixing or explaining. Thanks!

@Ema4rl
Copy link
Contributor

Ema4rl commented Jul 31, 2016

+1 👍
I support this.

@narfbg
Copy link
Contributor

narfbg commented Jan 5, 2022

Sorry all that I've left this one to rot. The Profiler has always been just an uninteresting chore for me.

@gxgpet I don't know if there are still people interested, but feel free to make any decision on this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants