Display the time Rails spends rendering a cell#6515
Merged
Leusev merged 18 commits intodecidim:developfrom Oct 14, 2020
alecslupu-pfa:feature/add-cell-instrumenter
Merged
Display the time Rails spends rendering a cell#6515Leusev merged 18 commits intodecidim:developfrom alecslupu-pfa:feature/add-cell-instrumenter
Leusev merged 18 commits intodecidim:developfrom
alecslupu-pfa:feature/add-cell-instrumenter
Conversation
Sync with master repo
…re/add-cell-instrumenter
Contributor
Author
|
@decidim/core, is there any chance you can review and approve this PR ? |
Contributor
Author
|
Update core decidim
regular update
Leusev
reviewed
Oct 6, 2020
Contributor
Leusev
left a comment
There was a problem hiding this comment.
Hi @alecslupu , glad to see you here again!
First of all, there's a [CI] Meetings flaky test failing in this PR, yet solved in PR #6595
By the moment, could you merge with develop in order to solve it please?
Thanks in advance!
Contributor
|
Btw, as now all cells display its render time in log, what do you think about it @decidim/core @tramuntanal @ivan-mr it could be interesting to add it ? |
12 tasks
Contributor
|
Hello @alecslupu |
Bump develop to next release version
Contributor
Author
|
@Leusev , the merge with develop is done |
Contributor
|
Hi yes I find it interesting as long as it does not penalize in production environments @Leusev |
Contributor
|
Good morning @alecslupu |
Contributor
|
@Leusev the failing test is that "random one" from meetings. The problem with the maps has been going on for a while now and is not caused by this PR... @alecslupu heads-up, some conflicts arose! |
Contributor
Author
@Leusev , on the same telegram chat i was telling you that if run only the file that fails in suite, the test will pass without any issues. Also, i see that currently in meetings module there is another flacky test .. the number of meetings found in filter. |
Contributor
|
Ok @alecslupu, you're right, the [CI] Meetings test is a flaky. |
…-cell-instrumenter
Contributor
Author
Thanks . Pushed the fix . |
Leusev
approved these changes
Oct 14, 2020
Contributor
Leusev
left a comment
There was a problem hiding this comment.
Ok, all ok now @alecslupu 👍
Thanks a lot for your contribution
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎩 Adding instrumentation method to display the time rails spends rendering a cell. Now the logs will display following information.
📌 Related Issues
📋 Subtasks
CHANGELOGupgrade notes, if required📷 Screenshots (optional)