util/generate: Use the same template for all generations#885
util/generate: Use the same template for all generations#885coriolinus merged 2 commits intoexercism:masterfrom
Conversation
coriolinus
left a comment
There was a problem hiding this comment.
Looks good, thanks @dvermd!
I know that the extern crate change is distinct from the primary intent of this PR. Therefore I do not think we should block this PR on that change. However, I would look favorably on adding a commit which removed the generated extern crate lines.
I will now give this PR a few days to ensure that other maintainers have had time to review and comment. If there is no objection, I intend to merge this on Friday.
|
|
||
| test_file.write_all( | ||
| &format!( | ||
| "extern crate {escaped_exercise_name};\n\ |
There was a problem hiding this comment.
As this track now uses the 2018 edition by default, we can remove the extern crate line.
There was a problem hiding this comment.
I'll remove that.
Following this advice, what about also changing
#[macro_use]
extern crate maplit;
to
use maplit::*;
|
It's not a bad idea to remove the extern crate, but we shouldn't be
encouraging people to use global imports. I think
use maplit::maplit;
will do what we need, but that'll need to be tested.
…On Tue, Oct 8, 2019 at 1:08 PM dvermd ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In util/exercise/src/cmd/generate.rs
<#885 (comment)>:
> + //! \n\
+ //! Generated by [utility][utility]\n\
+ //! \n\
+ //! [utility]: https://github.com/exercism/rust/tree/master/util/exercise\n\
+ \n",
+ exercise_name = exercise_name,
+ )
+ .into_bytes(),
+ )?;
+ if use_maplit {
+ test_file.write_all(b"#[macro_use]\nextern crate maplit;\n")?;
+ }
+
+ test_file.write_all(
+ &format!(
+ "extern crate {escaped_exercise_name};\n\
I'll remove that.
Following this advice, what about also changing
#[macro_use]
extern crate maplit;
to
use maplit::*;
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#885?email_source=notifications&email_token=AB3V4TWJTTRSZ7LXM4OJA5TQNRS25A5CNFSM4I6KO62KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCHGZ5MQ#discussion_r332454665>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AB3V4TQBGI4VZHPJ4VG26FLQNRS25ANCNFSM4I6KO62A>
.
|
|
A quick example from maplit shows that Right now, there's 5 macros in the maplit crate. I might import them all explicitly and then Another point : should I remove one of the "[utility][utility]" in the template ? I didn't manage to find documentation on a specific behaviour related to "[]" in doc comments. |
|
use maplit::hashmap;
is correct. That was my mistake earlier. I don't think the generator uses
any other literals, so we should not import the others.
You should not remove the "[utility][utility]"; that's a markdown external
hyperlink, which inlines the link which appears lower in the comment.
…On Tue, Oct 8, 2019 at 1:48 PM dvermd ***@***.***> wrote:
A quick example from maplit shows that use maplit::maplit; is not working
and one should write use maplit::hashmap; if the hashmap! macro is to be
used.
Right now, there's 5 macros in the maplit crate. I might import them all
explicitly and then cargo build will warn about unused imports.
I'm not happy with this but this is the best I can think right now.
Another point : should I remove one of the "[utility][utility]" in the
template ? I didn't manage to find documentation on a specific behaviour
related to "[]" in doc comments.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#885?email_source=notifications&email_token=AB3V4TXC3NOZMCZDZS6JSP3QNRXRRA5CNFSM4I6KO62KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAT4DRA#issuecomment-539476420>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AB3V4TWXXKCWQKCW2PRJVCTQNRXRRANCNFSM4I6KO62A>
.
|
This PR also do not repeat the test name creation.
Resolves: #679