Opened 3 years ago

Closed 3 years ago

#19564 closed defect (fixed)

Remove pointers from boost_interface

Reported by: ncohen Owned by:
Priority: major Milestone: sage-6.10
Component: graph theory Keywords:
Cc: borassi, dcoudert Merged in:
Authors: Nathann Cohen Reviewers: Michele Borassi
Report Upstream: N/A Work issues:
Branch: 6d81760 (Commits) Commit: 6d81760a88ec4ea8a7688f34b498dd01263b01ab
Dependencies: Stopgaps:

Description

It seemed to me that those pointers were not required in any way. Sooo unless I am wrong, let's get rid of them!

Nathann

Change History (10)

comment:1 Changed 3 years ago by ncohen

  • Branch set to public/19564
  • Commit set to 6d81760a88ec4ea8a7688f34b498dd01263b01ab
  • Status changed from new to needs_review

New commits:

2b73473*graph -> graph
6d81760*vertices -> vertices

comment:2 Changed 3 years ago by borassi

  • Reviewers set to Michele Borassi

I have tested the whole Sage library, and it works perfectly. Indeed, I have no idea why I used pointers in the first place: maybe it was an attempt to solve a bug, that later I did not undo.

For me, this patch is good to go, if the bot agrees (it seems that the startup time is not fine). Let me know if you solve this problem, and I will set it to positive review.

comment:3 Changed 3 years ago by chapoton

As always, the patchbot results have to be human-interpreted. Here one the patchbots does not complain about startup time, so there is no real problem.

The startup plugin is doing some statistics on the startup time, and this is rather flaky. So a failure of this plugin should rather be taken as "please check twice if you do not do something bad involving the startup time", IMHO.

comment:4 Changed 3 years ago by ncohen

Would it be possible to reduce the amount of 'false positive', by only reporting the test if we have more certitude that something was slowed down?

One doesn't look at a warning which is raised too often uselessly.

Nathann

comment:5 Changed 3 years ago by chapoton

I have not the least idea of the inner gears of this plugin, and I will not change it myself.

comment:6 Changed 3 years ago by ncohen

Do you know who wrote it, so that I may forward this 'bug report'?

comment:8 Changed 3 years ago by ncohen

  • Status changed from needs_review to positive_review

Thanks.

I'm setting this ticket to positive_review, as Michele reviewed it and only kept his hand from changing the status because of that warning.

Nathann

comment:9 Changed 3 years ago by borassi

Great!

comment:10 Changed 3 years ago by vbraun

  • Branch changed from public/19564 to 6d81760a88ec4ea8a7688f34b498dd01263b01ab
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.