Skip to content

Conversation

@ChrisJefferson
Copy link
Contributor

Part of #5261 , which removes a function which was previously tested in the profiling package.

@fingolfin fingolfin added the release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes label Jan 2, 2023
@fingolfin fingolfin changed the title Remove ActivateProfileColour Remove ActivateProfileColour (it was rarely used or even known, and blocked work on the profiling engine) Jan 2, 2023
return Fail;
}

ActivatePrintHooks(&profilePrintHooks);
Copy link
Member

Choose a reason for hiding this comment

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

Is it worth it to keep the whole print hook machinery in place?

** see this when we use ActivateProfileColour. However, without some
** serious additional overheads, I can't see how to provide this
** 3) Operating at just a line basis can sometimes be too course. However,
** without some serious additional overheads, I can't see how to provide this
Copy link
Member

Choose a reason for hiding this comment

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

There is another reference to colo(u)r in line 66. Also, the variable ColouringOutput is not actually removed.

I am going to update this PR to fix this and my other remarks.

@fingolfin fingolfin force-pushed the remove_profcolour branch 2 times, most recently from c3ade9b to e173e3a Compare January 2, 2023 11:58
@fingolfin fingolfin enabled auto-merge (rebase) January 4, 2023 22:44
@fingolfin fingolfin merged commit cbc1a6f into gap-system:master Jan 4, 2023
@ChrisJefferson ChrisJefferson deleted the remove_profcolour branch December 18, 2023 07:10
@fingolfin fingolfin added the kind: removal or deprecation A feature was removed or deprecated / made obsolete label Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind: removal or deprecation A feature was removed or deprecated / made obsolete release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants