Skip to content

Conversation

@rvanvenetie
Copy link
Owner

No description provided.

@rvanvenetie
Copy link
Owner Author

I don't think we should merge this.

Copy link
Collaborator

@Jannertje Jannertje left a comment

Choose a reason for hiding this comment

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

Ik denk dat hier ook wel changes tussen zitten die nuttig zijn ookal gaat deze PR er niet in: als we compute_fibers direct aanroepen, hoef je fibers_ niet mutable te maken en hebben we iets meer controle over dat stukje code. Zo zijn er nog wel wat stukjes die verdedigbaar zijn; loop er nog 's doorheen en als je iets tegenkomt wat je wil houden, stuur maar een PR :-)
Ik zou deze branch wel willen bewaren trouwens.

@rvanvenetie
Copy link
Owner Author

Ik denk dat hier ook wel changes tussen zitten die nuttig zijn ookal gaat deze PR er niet in: als we compute_fibers direct aanroepen, hoef je fibers_ niet mutable te maken en hebben we iets meer controle over dat stukje code. Zo zijn er nog wel wat stukjes die verdedigbaar zijn; loop er nog 's doorheen en als je iets tegenkomt wat je wil houden, stuur maar een PR :-)
Ik zou deze branch wel willen bewaren trouwens.

Ja, dat compute_fibers was nodig om race conditions te voorkomen, misschien leuk in de toekomst inderdaad. Ja, ik dacht eraan om deze branch gewoon te houden, en dan kunnen we changes terugmergen in de toekomst.

@rvanvenetie rvanvenetie changed the base branch from cpp/master to master March 15, 2021 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants