Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feature] add odt comment #618

Merged
merged 11 commits into from
May 4, 2020
Merged

Conversation

patochectp
Copy link
Member

No description provided.

Copy link
Contributor

@ArnaudOggy ArnaudOggy left a comment

Choose a reason for hiding this comment

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

You can already prepare the reception normally

/// OnDemandTransport comment
#[structopt(long)]
odt_comment: Option<String>,

In gtfs2ntfs/src/main.rs
and gtfs2netexfr/src/main.rs

@patochectp
Copy link
Member Author

You can already prepare the reception normally

/// OnDemandTransport comment
#[structopt(long)]
odt_comment: Option<String>,

In gtfs2ntfs/src/main.rs
and gtfs2netexfr/src/main.rs

Done

@patochectp patochectp closed this Apr 28, 2020
@patochectp patochectp reopened this Apr 28, 2020
@patochectp patochectp removed the wip label Apr 30, 2020
Copy link
Contributor

@woshilapin woshilapin left a comment

Choose a reason for hiding this comment

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

Two minor comments, apart from that, nice refactorization.

@@ -36,7 +36,7 @@ struct Opt {

/// config file
#[structopt(short = "c", long = "config", parse(from_os_str))]
Copy link
Contributor

Choose a reason for hiding this comment

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

Then you should be able to do this now.

Suggested change
#[structopt(short = "c", long = "config", parse(from_os_str))]
#[structopt(short, long, parse(from_os_str))]

pub on_demand_transport_comment: Option<String>,
}

fn read<H>(file_handler: &mut H, configuration: Configuration<impl AsRef<Path>>) -> Result<Model>
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting, any specific reason that made you choose

fn read<H>(file_handler: &mut H, configuration: Configuration<impl AsRef<Path>>) -> Result<Model>

over something like

fn read<H, P: AsRef<Path>>(file_handler: &mut H, configuration: Configuration<P>) -> Result<Model>

Copy link
Member

Choose a reason for hiding this comment

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

apparently it's the same with a small difference https://stackoverflow.com/a/47515540

Copy link
Contributor

Choose a reason for hiding this comment

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

Excellent link, thank you.

@patochectp
Copy link
Member Author

Two minor comments, apart from that, nice refactorization.

Thanks for your help !

@datanel datanel merged commit c9d6092 into hove-io:master May 4, 2020
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.

4 participants