[#224] Add description for multiline puzzle #237

pnatashap · 2024-01-21T03:29:49Z

Examples for multiline puzzles
Remove unsupported examples

Fix #224

…d formats

pnatashap · 2024-01-21T03:31:13Z

@php-coder @yegor256 please take a look

yegor256 · 2024-01-21T07:32:24Z

@pnatashap looks good to me. @php-coder WDYT?

README.md

php-coder · 2024-01-22T05:10:02Z

README.md

@@ -172,13 +176,11 @@ as long as you have one of the 3 supported keywords right in front
 of the mandatory marker):

 ```
-// @todo #224
-/* @todo #TEST-13 */


Why it was removed?

It is not supported in the code, there is only a logic to remove prefix, but no logic to remove any kinds of symbols from the end

In this case, I'd suggest to fix/improve support for such type of comments instead of removing it from documentation.

There is a bug about it #225
I think it is better to remove it now and return it back after fix

@pnatashap If we remove this examples, I believe we have to remove TEST-13 and 42 from the next sentence too:

224, TEST-13, 55, 67, 678, 1, and 42 are the IDs of the tickets these puzzles are coming from.

php-coder · 2024-01-22T05:10:21Z

README.md

-@todo #67/DES
-;; @todo #678:40m/DEV
-// TODO: #1:30min
-(* TODO #42 *)


The same: why it has been removed?

It is not supported in the code, there is only a logic to remove prefix, but no logic to remove any kinds of symbols from the end

README.md

php-coder · 2024-01-22T05:18:31Z

README.md

+ * @todo #36 Multiline text can use the same prefix in all lines or the same 
+ *  amount of spaces.
+    So this will be added to the puzzle description. If you want to divide the 
+ *  puzzle by empty line, just add a space to that line  


IMHO we should reconsider and improve this rule. Using a trailing space for making decisions isn't good.
First, it's non-obvious.
Second, it's invisible, so it's really hard to catch such issues on code review.
Third, users may want to run auto-formatting tools, that can remove such "noise". In this case, users will face a massive close and creation of the issues.

How about using a backslash? Like this:

/** * @todo #36 Multiline text can use the same prefix in all lines or the same * amount of spaces. So this will be added to the puzzle description. If you want to divide the * puzzle by empty line, just add a backslash to that line * \ * and continue the text after. * * This line is not part of the puzzle, because the line before contains less * spaces then the second line. */

@yegor256 WDYT?

@pnatashap @php-coder indeed, a "blind" space at the end of a line is provocation of a mistake

Good symbol should be clear enough for developers.
Also will check if it works for second line as described in #163

php-coder · 2024-01-22T05:19:50Z

Also, I suggest to add Fix #224 to the PR description, so the GitHub will close the related issue once the PR gets merged.

Co-authored-by: Slava Semushin <[email protected]>

yegor256 · 2024-01-25T05:24:10Z

@pnatashap please, make the fixed suggested and then we'll merge this one

pnatashap · 2024-01-27T04:05:09Z

@php-coder @yegor256 please take a look

php-coder · 2024-01-29T06:09:37Z

LGTM

But it breaks backward compatibility :( Ideally, we should keep it and accept both styles some time simultaneously with a warning about usage of old-style comments. But as I'm not using 0pdd anymore, so now I don't care about that much as I my repos won't have new issues created and the old one closed because of this change...

yegor256 · 2024-01-29T08:16:17Z

@volodya-lombrozo can you please take a look at this one. Do you see any problems?

pnatashap · 2024-01-29T16:33:24Z

LGTM

But it breaks backward compatibility :( Ideally, we should keep it and accept both styles some time simultaneously with a warning about usage of old-style comments. But as I'm not using 0pdd anymore, so now I don't care about that much as I my repos won't have new issues created and the old one closed because of this change...

Actually space as a non breaking separator was a change from previous week and not released (and was not presented in documentation) so it is not about backward capability.
Removed one-line examples just not supported (I do not see any test for them)

volodya-lombrozo · 2024-01-31T12:33:21Z

README.md

@@ -172,13 +176,11 @@ as long as you have one of the 3 supported keywords right in front
 of the mandatory marker):

 ```
-// @todo #224
-/* @todo #TEST-13 */


@pnatashap If we remove this examples, I believe we have to remove TEST-13 and 42 from the next sentence too:

224, TEST-13, 55, 67, 678, 1, and 42 are the IDs of the tickets these puzzles are coming from.

volodya-lombrozo · 2024-01-31T12:45:54Z

README.md

+
+```xml
+<!-- 
+ ~ if comment should be started and closed by special symbols, then place them in


@pnatashap What Is "prefix" here? ~ ? I haven't found the definition of "prefix" in the README file.

It can be used to decorate text, not required, but for there is no restrictions about the prefix, it should be just equal for all text. So it is just an example

@pnatashap, maybe we can add this to the documentation? I mean, we can state that the "prefix" is a symbol that can be used to decorate text; it isn't required. And provide some examples.

Moreover, I would suggest adding some headers or small - one-sentence descriptions to these code examples:

"...Prefix definition... Here we ignore the prefix, and only one puzzle will be created."

"We can also use the \ symbol to connect multiline puzzles."

What do you think?

Remove task numbers from the result description

yegor256 · 2024-02-01T03:27:10Z

@pnatashap something wrong here with the codecov task?

pnatashap · 2024-02-01T03:30:20Z

There is a error on report upload
[2024-01-31T17:00:02.755Z] ['error'] There was an error running the uploader: Error uploading to [https://codecov.io:](https://codecov.io/) Error: There was an error fetching the storage URL during POST: 404 - {'detail': ErrorDetail(string='Unable to locate build via Github Actions API. Please upload with the Codecov repository upload token to resolve issue.', code='not_found')}
Rerun button is not available for me, but it should work, as report created correctly

codecov · 2024-02-01T04:33:36Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8db7aaa) 95.50% compared to head (317b976) 95.69%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #237      +/-   ##
==========================================
+ Coverage   95.50%   95.69%   +0.19%     
==========================================
  Files          10       10              
  Lines         356      372      +16     
==========================================
+ Hits          340      356      +16     
  Misses         16       16

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pnatashap · 2024-02-01T04:38:38Z

codecov-action@v4 solves the issue with upload...
will check why it is decreased :) Check code coverage on PR creation in action

volodya-lombrozo · 2024-02-01T08:01:07Z

features/step_definitions/steps.rb

@@ -83,10 +83,6 @@
  raise "STDOUT doesn't contain '#{txt}':\n#{@stdout}" unless @stdout.include?(txt)
 end

-Then(/^Stdout is empty$/) do


@pnatashap, I don't understand why you decided to remove some steps from steps.rb. The report states the following:

28 scenarios (23 undefined, 5 passed) 138 steps (34 skipped, 28 undefined, 76 passed)

However, we used to have:

28 scenarios (28 passed) 138 steps (138 passed)

Maybe it's worth keeping all the scenarios passing?

Locally it was displayed, that it is duplicated steps and should be removed. Forget that they do not fail. Returned them back

A flag is added to cucumber run to fail the task in case of undefined step

volodya-lombrozo · 2024-02-01T08:14:47Z

README.md

+
+```xml
+<!-- 
+ ~ if comment should be started and closed by special symbols, then place them in


@pnatashap, maybe we can add this to the documentation? I mean, we can state that the "prefix" is a symbol that can be used to decorate text; it isn't required. And provide some examples.

Moreover, I would suggest adding some headers or small - one-sentence descriptions to these code examples:

"...Prefix definition... Here we ignore the prefix, and only one puzzle will be created."

"We can also use the \ symbol to connect multiline puzzles."

What do you think?

volodya-lombrozo

@pnatashap Looks good to me. Thank you!

pnatashap · 2024-02-04T15:29:06Z

@yegor256 please take a look

yegor256 · 2024-02-04T16:19:12Z

@rultor merge

rultor · 2024-02-04T16:21:53Z

@rultor merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

rultor · 2024-02-04T16:25:24Z

@rultor merge

@yegor256 Done! FYI, the full log is here (took me 3min)

[cqfn#224] Add description for multiline puzzle and remove unsupporte…

2f83a39

…d formats

php-coder reviewed Jan 22, 2024

View reviewed changes

README.md Outdated Show resolved Hide resolved

php-coder reviewed Jan 22, 2024

View reviewed changes

README.md Outdated Show resolved Hide resolved

php-coder reviewed Jan 22, 2024

View reviewed changes

pnatashap and others added 2 commits January 22, 2024 14:07

Update README.md

8e26e6f

Co-authored-by: Slava Semushin <[email protected]>

Update README.md

a2acecf

Co-authored-by: Slava Semushin <[email protected]>

[cqfn#224] Change space symbol to backslash for newline

0c10776

[cqfn#224] Add a test for javadoc and small change

194a4ba

volodya-lombrozo suggested changes Jan 31, 2024

View reviewed changes

Update README.md

a71e657

Remove task numbers from the result description

pnatashap added 2 commits January 31, 2024 23:02

[cqfn#238] Add another example for ticket id's

291030b

[cqfn#238] Sink with master and update codecov-action (just released)

bb3f370

[cqfn#238] Add a test for last empty line

438d324

pnatashap requested review from volodya-lombrozo and yegor256 February 1, 2024 04:59

volodya-lombrozo suggested changes Feb 1, 2024

View reviewed changes

pnatashap added 3 commits February 1, 2024 09:27

[cqfn#238] Return back steps

4e4fe39

[cqfn#238] Flag to fail Cucumber tests in case of missing steps

53b9be5

[cqfn#238] Changed description

317b976

pnatashap requested a review from volodya-lombrozo February 2, 2024 05:27

volodya-lombrozo approved these changes Feb 2, 2024

View reviewed changes

rultor merged commit 317b976 into cqfn:master Feb 4, 2024
8 checks passed

pnatashap deleted the 224 branch February 4, 2024 16:50

[#224] Add description for multiline puzzle #237

[#224] Add description for multiline puzzle #237

Conversation

pnatashap commented Jan 21, 2024 • edited Loading

pnatashap commented Jan 21, 2024

yegor256 commented Jan 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pnatashap Jan 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

php-coder commented Jan 22, 2024

yegor256 commented Jan 25, 2024

pnatashap commented Jan 27, 2024

php-coder commented Jan 29, 2024

yegor256 commented Jan 29, 2024

pnatashap commented Jan 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yegor256 commented Feb 1, 2024

pnatashap commented Feb 1, 2024

codecov bot commented Feb 1, 2024 • edited Loading

Codecov Report

pnatashap commented Feb 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pnatashap Feb 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

volodya-lombrozo left a comment

Choose a reason for hiding this comment

pnatashap commented Feb 4, 2024

yegor256 commented Feb 4, 2024

rultor commented Feb 4, 2024

rultor commented Feb 4, 2024

pnatashap commented Jan 21, 2024 •

edited

Loading

pnatashap Jan 27, 2024 •

edited

Loading

codecov bot commented Feb 1, 2024 •

edited

Loading

pnatashap commented Feb 1, 2024 •

edited

Loading

pnatashap Feb 1, 2024 •

edited

Loading