First remarks on code - Githubissues

hove-io / transit_model

Managing transit data with Rust

GNU Affero General Public License v3.0

56 stars 28 forks source link

First remarks on code #1

Closed pbougue closed 6 years ago

pbougue commented 6 years ago

@TeXitoi To be discussed, as there's no PR to discuss on.

https://github.com/CanalTP/navitia_model/blob/9327364e29645194cb79866609131e657dff2be8/src/lib.rs#L79 I'd name it Context more than PtObjects
https://github.com/CanalTP/navitia_model/blob/9327364e29645194cb79866609131e657dff2be8/src/lib.rs#L10 :) not able to read or maintain that... We have to take a course on that! Maybe clearer (longer) macro param names (what is ty for?)?
https://github.com/CanalTP/navitia_model/blob/09d7b0d4913b8df7469977c7fa0d4e8baf5b60e5/get-corresponding-derive/src/lib.rs#L12 Again the naming is not super helping, or missing comments (also the case for get-corresponding-derive/src/lib.rs in general)?

TeXitoi commented 6 years ago

As you wish, it was named like that when the crate was named pt_objects.
Funny, I removed this macro this morning before reading this issue https://github.com/CanalTP/navitia_model/commit/d90366cf95438b6e403054dbb4fd7492d2a47bc6
https://doc.rust-lang.org/book/first-edition/procedural-macros.html (ast if for Abstract Syntax Tree).