rna-transcription: abbreviate to 'rna' and 'dna'#410
rna-transcription: abbreviate to 'rna' and 'dna'#410petertseng merged 4 commits intoexercism:masterfrom shingtaklam1324:master
Conversation
petertseng
left a comment
petertseng
left a comment
There was a problem hiding this comment.
Reference Issues
Please specifically use one of the words listed in https://help.github.com/articles/closing-issues-using-keywords/ so that the issue will be closed when this is merged.
Also ran the example code and tests through rustfmt
I would normally say this should be in its own commit and not merge this PR until that is done, but since most lines changed where also changed by the RNA/DNA change, it is mostly fine. I pointed out four lines where the change does not coincide with an RNA/DNA change.
The sources for what I say are:
- The guidelines of https://github.com/exercism/docs/blob/master/contributing/pull-request-guidelines.md where it says "should not include the word and (explicitly or implied)."
- https://git-scm.com/docs/gitworkflows/#_separate_changes
Since in most cases it coincides, I won't argue if someone else decides it is OK to Approve and merge this. For Approval from me specifically, I need the rustfmt change to be in its own commit. Note that it is not necessary to have Approval from me specifically.
| } | ||
|
|
||
| pub fn to_rna(&self) -> Result<RibonucleicAcid, ()> { | ||
| let rna_nucleotides: String = self.nucleotides.chars().filter_map(transcribe_dna_rna).collect(); |
There was a problem hiding this comment.
unrelated line change caused by fmt.
| 'A' => Some('U'), | ||
| 'T' => Some('A'), | ||
| _ => None | ||
| _ => None, |
There was a problem hiding this comment.
unrelated line change caused by fmt.
| pub struct DeoxyribonucleicAcid { | ||
| nucleotides: String | ||
| pub struct DNA { | ||
| nucleotides: String, |
There was a problem hiding this comment.
unrelated line change caused by fmt.
| pub struct RibonucleicAcid { | ||
| nucleotides: String | ||
| pub struct RNA { | ||
| nucleotides: String, |
There was a problem hiding this comment.
unrelated line change caused by fmt.
|
Un- |
petertseng
changed the title
petertseng
left a comment
petertseng
left a comment
There was a problem hiding this comment.
As far as I can tell, this PR now contains only the rename to DNA and RNA. Since this approach gained acceptance in the issue proposing it, I think I'll merge it (I will squash the commits and edit the message to use one of the necessary words) after the tests pass.
|
Thank you. Feel free to submit a PR for rustfmt changes if you like. One commit that |
|
Thanks, given that none of the rest of the repository uses |
|
For #143 , |
2 participants
Also ran the example code and tests through
rustfmtReference Issues
#407