Skip to content

Conversation

@fingolfin
Copy link
Member

  • NamesLocalVariablesFunction actually is a kernel operation, mark it as such
  • fix a crash caused by infinite recursion when trying to display a function
    which is not a T_FUNCTION
  • add tests for installing custom methods for NameFunction,
    NamesLocalVariablesFunction, and NumberArgumentsFunction

@fingolfin fingolfin added the kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements label Apr 30, 2018
Copy link
Contributor

@ThomasBreuer ThomasBreuer left a comment

Choose a reason for hiding this comment

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

This is an improvement.

Concerning the change in lib/function.g, also the documentation for some of the operations could be corrected (replace <Func Name=... by <Oper Name=..., replace #F by #O, remove the #T lines that state that a function should better be an operation).

Concerning the new tests whether methods for the operations can be installed, what about tests that the installed methods get called?

* NamesLocalVariablesFunction actually is a kernel operation, mark it as such
* fix a crash caused by infinite recursion when trying to display a function
  which is not a `T_FUNCTION`
* add tests for installing custom methods for NameFunction,
  NamesLocalVariablesFunction, and NumberArgumentsFunction
@codecov
Copy link

codecov bot commented May 1, 2018

Codecov Report

Merging #2427 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2427      +/-   ##
==========================================
+ Coverage   73.88%   73.89%   +<.01%     
==========================================
  Files         484      484              
  Lines      245465   245465              
==========================================
+ Hits       181371   181375       +4     
+ Misses      64094    64090       -4
Impacted Files Coverage Δ
lib/function.gi 77.58% <ø> (ø) ⬆️
lib/function.g 32.3% <ø> (ø) ⬆️
src/hpc/threadapi.c 43.3% <0%> (-0.11%) ⬇️
src/calls.c 94.67% <0%> (+0.45%) ⬆️
hpcgap/lib/hpc/stdtasks.g 64.96% <0%> (+0.51%) ⬆️

@fingolfin
Copy link
Member Author

@ThomasBreuer good remarks, thanks, hopefully all addressed now (also the test failure in HPC-GAP)

@ChrisJefferson ChrisJefferson merged commit 5908729 into gap-system:master May 2, 2018
@fingolfin fingolfin deleted the mh/custom-funcs branch May 2, 2018 14:18
@fingolfin fingolfin added the release notes: added PRs introducing changes that have since been mentioned in the release notes label Sep 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: added PRs introducing changes that have since been mentioned in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants