Opened 5 years ago
Closed 5 years ago
#24153 closed enhancement (fixed)
Remove unused functions from ccobject.h
Reported by:  Jeroen Demeyer  Owned by:  

Priority:  major  Milestone:  sage8.1 
Component:  cython  Keywords:  
Cc:  Travis Scrimshaw  Merged in:  
Authors:  Jeroen Demeyer  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  fdffc30 (Commits, GitHub, GitLab)  Commit:  fdffc309c6c9c588e4e01eb6e00b1f8704b3a3eb 
Dependencies:  Stopgaps: 
Description
Thanks to various C++ cleanup tickets, most of ccobject.h
is obsolete now.
Change History (5)
comment:1 Changed 5 years ago by
Branch:  → u/jdemeyer/ticket/24153 

comment:2 Changed 5 years ago by
Commit:  → fdffc309c6c9c588e4e01eb6e00b1f8704b3a3eb 

Status:  new → needs_review 
comment:3 Changed 5 years ago by
Reviewers:  → Travis Scrimshaw 

I know this is bikeshedding, nobody will probably ever really read this file, and C++ does not have nearly the same style uniformity that Python has. So you can completely ignore this. Yet, IMO, most C++ programmers have their opening {
on the same line that starts the block (and it is more Pythonesque). So I do not quite agree with this:

src/sage/ext/ccobject.h
diff git a/src/sage/ext/ccobject.h b/src/sage/ext/ccobject.h index 6753a0a..c6cadb9 100644
a b static CYTHON_INLINE PyObject* ccrepr(const T& x) 114 49 #endif 115 50 } 116 51 52 117 53 /* Arrays */ 118 54 template <class T> 119 static inline T* Allocate_array(size_t n){ 120 return new T[n]; 55 static inline T* Allocate_array(size_t n) 56 { 57 return new T[n]; 121 58 } 122 59 123 60 template <class T> 124 static inline void Delete_array(T* v){ 125 delete[] v; 61 static inline void Delete_array(T* v) 62 { 63 delete[] v; 126 64 } 127 65 128 66 #endif
However, if you prefer it in even the slightest (including out of laziness), then just set a positive review. Likewise, if you undo this change, you can set a positive review on my behalf.
comment:4 Changed 5 years ago by
Status:  needs_review → positive_review 

Google agrees with you: http://google.github.io/styleguide/cppguide.html#Function_Declarations_and_Definitions
GNU agrees with me: https://www.gnu.org/prep/standards/standards.html#Formatting
I'll go for the "lazy" option and keep this branch as is.
comment:5 Changed 5 years ago by
Branch:  u/jdemeyer/ticket/24153 → fdffc309c6c9c588e4e01eb6e00b1f8704b3a3eb 

Resolution:  → fixed 
Status:  positive_review → closed 
New commits:
Remove unused functions from ccobject.h