Skip to content
This repository has been archived by the owner on Mar 22, 2022. It is now read-only.

configlet fmt on deprecated exercises: Should not re-add core, unlocked_by, difficulty, topics #140

Open
petertseng opened this issue Jun 12, 2018 · 1 comment · May be fixed by #169
Open

Comments

@petertseng
Copy link
Member

recent configlet fmt adds these keys to all exercises in the deprecated array

+      "core": false,
+      "unlocked_by": null,
+      "difficulty": 0,
+      "topics": null,

In exercism/discussions#159 (comment) I argued that I saw no reason to keep difficulty and topics there. I continue to see no reason, however this is your chance to make me see a reason.

For example, perhaps any exercise that becomes deprecated could simply retain all its old values for these fields. And perhaps that is eminently useful, for some reason. That would certainly be cause for me to see a reason.

In such a case, please be advised that I'm not going to expend mental energy on making up values for exercises that were deprecated before this operation, so these values are going to remain whatever values configlet fmt decided to place for them (0 and null respectively), and I hope that's okay.

I also reject a workflow that manually removes these keys from deprecated exercises after having run configlet fmt because 1) why would any person want to do manual work 2) https://github.com/exercism/haskell/blob/master/bin/check-configlet-fmt.sh https://github.com/exercism/rust/blob/master/_test/check-configlet-fmt.sh would make Travis CI reject it anyway.

It may be possible to exclude those fields from the deprecated array only, but that would involve either keeping two structs in sync painstakingly, or some embedding, and I have not yet speculated on the clarity of the resulting structs. I cannot advise on this course of action.

If something else should happen, please advise.

@kytrinyx
Copy link
Member

I continue to see no reason, however this is your chance to make me see a reason.

I agree with you. I had forgotten to add tests for this case.

@petertseng petertseng changed the title configlet fmt on deprecated exercises: Please advise on core, unlocked_by, difficulty, topics configlet fmt on deprecated exercises: Should not re-add core, unlocked_by, difficulty, topics Apr 24, 2019
petertseng referenced this issue in petertseng/configlet Sep 17, 2019
We don't need any of the other fields, and it's deceptive to keep them
in the JSON file because a maintainer may mistakenly believe them to
mean something, whereas they mean nothing.

Two new tests were added: TestMarshalActive and TestMarshalDeprecated.
TestMarshalActive passes both before and after the associated code
change.
TestMarshalDeprecated fails before the associated code change, and
passes after.

Closes exercism/configlet#140
petertseng referenced this issue in petertseng/configlet Sep 17, 2019
We don't need any of the other fields, and it's deceptive to keep them
in the JSON file because a maintainer may mistakenly believe them to
mean something, whereas they mean nothing.

Two new tests were added: TestMarshalActive and TestMarshalDeprecated.
TestMarshalActive passes both before and after the associated code
change.
TestMarshalDeprecated fails before the associated code change, and
passes after.

Closes exercism/configlet#140
petertseng referenced this issue in petertseng/configlet Sep 17, 2019
We don't need any of the other fields, and it's deceptive to keep them
in the JSON file because a maintainer may mistakenly believe them to
mean something, whereas they mean nothing.

Two new tests were added: TestMarshalActive and TestMarshalDeprecated.
TestMarshalActive passes both before and after the associated code
change.
TestMarshalDeprecated fails before the associated code change, and
passes after.

Closes exercism/configlet#140
petertseng referenced this issue in petertseng/configlet Sep 17, 2019
We don't need any of the other fields, and it's deceptive to keep them
in the JSON file because a maintainer may mistakenly believe them to
mean something, whereas they mean nothing.

Two new tests were added: TestMarshalActive and TestMarshalDeprecated.
TestMarshalActive passes both before and after the associated code
change.
TestMarshalDeprecated fails before the associated code change, and
passes after.

Closes exercism/configlet#140
petertseng referenced this issue in petertseng/configlet Sep 17, 2019
We don't need any of the other fields, and it's deceptive to keep them
in the JSON file because a maintainer may mistakenly believe them to
mean something, whereas they mean nothing.

Two new tests were added: TestMarshalActive and TestMarshalDeprecated.
TestMarshalActive passes both before and after the associated code
change.
TestMarshalDeprecated fails before the associated code change, and
passes after.

Closes exercism/configlet#140
petertseng referenced this issue in petertseng/configlet Sep 17, 2019
We don't need any of the other fields, and it's deceptive to keep them
in the JSON file because a maintainer may mistakenly believe them to
mean something, whereas they mean nothing.

Two new tests were added: TestMarshalActive and TestMarshalDeprecated.
TestMarshalActive passes both before and after the associated code
change.
TestMarshalDeprecated fails before the associated code change, and
passes after.

Closes exercism/configlet#140
petertseng referenced this issue in petertseng/configlet Sep 17, 2019
We don't need any of the other fields, and it's deceptive to keep them
in the JSON file because a maintainer may mistakenly believe them to
mean something, whereas they mean nothing.

Two new tests were added: TestMarshalActive and TestMarshalDeprecated.
TestMarshalActive passes both before and after the associated code
change.
TestMarshalDeprecated fails before the associated code change, and
passes after.

Closes exercism/configlet#140
petertseng referenced this issue in petertseng/configlet Sep 17, 2019
We don't need any of the other fields, and it's deceptive to keep them
in the JSON file because a maintainer may mistakenly believe them to
mean something, whereas they mean nothing.

Two new tests were added: TestMarshalActive and TestMarshalDeprecated.
TestMarshalActive passes both before and after the associated code
change.
TestMarshalDeprecated fails before the associated code change, and
passes after.

Closes exercism/configlet#140
petertseng referenced this issue in petertseng/configlet Sep 18, 2019
We don't need any of the other fields, and it's deceptive to keep them
in the JSON file because a maintainer may mistakenly believe them to
be significant, but in fact they are not; the website does not display
ny of the removed fields:
https://github.com/exercism/website/blob/b3314e84c52b52cf5f253386c3f0bf4127ca09aa/app/views/my/tracks/_show_side_exercises.html.haml#L26-L34

Two new tests were added: TestMarshalActive and TestMarshalDeprecated.
TestMarshalActive passes both before and after the associated code
change.
TestMarshalDeprecated fails before the associated code change, and
passes after.

Closes exercism/configlet#140
petertseng referenced this issue in petertseng/configlet Sep 18, 2019
We don't need any of the other fields, and it's deceptive to keep them
in the JSON file because a maintainer may mistakenly believe them to
be significant, but in fact they are not; the website does not display
ny of the removed fields:
https://github.com/exercism/website/blob/b3314e84c52b52cf5f253386c3f0bf4127ca09aa/app/views/my/tracks/_show_side_exercises.html.haml#L26-L34

Two new tests were added: TestMarshalActive and TestMarshalDeprecated.
TestMarshalActive passes both before and after the associated code
change.
TestMarshalDeprecated fails before the associated code change, and
passes after.

Closes exercism/configlet#140
petertseng referenced this issue in petertseng/configlet Sep 18, 2019
We don't need any of the other fields, and it's deceptive to keep them
in the JSON file because a maintainer may mistakenly believe them to
be significant, but in fact they are not; the website does not display
any of the removed fields:
https://github.com/exercism/website/blob/b3314e84c52b52cf5f253386c3f0bf4127ca09aa/app/views/my/tracks/_show_side_exercises.html.haml#L26-L34

Two new tests were added: TestMarshalActive and TestMarshalDeprecated.
TestMarshalActive passes both before and after the associated code
change.
TestMarshalDeprecated fails before the associated code change, and
passes after.

Closes exercism/configlet#140
petertseng referenced this issue in petertseng/configlet Sep 18, 2019
We don't need any of the other fields, and it's deceptive to keep them
in the JSON file because a maintainer may mistakenly believe them to
be significant, but in fact they are not; the website does not display
any of the removed fields:
https://github.com/exercism/website/blob/b3314e84c52b52cf5f253386c3f0bf4127ca09aa/app/views/my/tracks/_show_side_exercises.html.haml#L26-L34

Two new tests were added: TestMarshalActive and TestMarshalDeprecated.
TestMarshalActive passes both before and after the associated code
change.
TestMarshalDeprecated fails before the associated code change, and
passes after.

Closes exercism/configlet#140
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants