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

Fix an issue with double comments #5824

Merged

Conversation

eivindjahren
Copy link
Contributor

@eivindjahren eivindjahren commented Aug 7, 2023

Issue
Resolves #5819
Resolves #5814

Pre review checklist

  • Added appropriate release note label
  • PR title captures the intent of the changes, and is fitting for release notes.
  • Commit history is consistent and clean, in line with the contribution guidelines.
  • Updated documentation
  • Ensured new behaviour is tested

Adding labels helps the maintainers when writing release notes. This is the list of release note labels.

@eivindjahren eivindjahren added the release-notes:bug-fix Automatically categorise as bug fix in release notes label Aug 7, 2023
@eivindjahren eivindjahren self-assigned this Aug 7, 2023
Copy link
Contributor

@andreas-el andreas-el left a comment

Choose a reason for hiding this comment

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

Nice work! 👍

@andreas-el andreas-el self-requested a review August 7, 2023 12:09
@eivindjahren eivindjahren force-pushed the fix_bug_with_double_comments branch 2 times, most recently from 0be94cc to 08cd717 Compare August 7, 2023 12:41
Copy link
Contributor

@andreas-el andreas-el left a comment

Choose a reason for hiding this comment

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

LGTM. I put my faith in the tests for the grammar. This surely will need iterations in the future due to incoming user feedback.

@eivindjahren eivindjahren force-pushed the fix_bug_with_double_comments branch 2 times, most recently from 5c2c6e2 to fe92b6d Compare August 7, 2023 13:17
@codecov-commenter
Copy link

Codecov Report

Merging #5824 (fe92b6d) into main (24ea4dc) will decrease coverage by 0.04%.
Report is 1 commits behind head on main.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #5824      +/-   ##
==========================================
- Coverage   81.47%   81.43%   -0.04%     
==========================================
  Files         346      346              
  Lines       21846    21843       -3     
  Branches      720      720              
==========================================
- Hits        17798    17787      -11     
- Misses       3795     3803       +8     
  Partials      253      253              
Files Changed Coverage Δ
src/ert/config/parsing/observations_parser.py 95.45% <ø> (ø)
src/ert/config/parsing/lark_parser.py 95.43% <100.00%> (-0.47%) ⬇️

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@eivindjahren eivindjahren force-pushed the fix_bug_with_double_comments branch 3 times, most recently from d93c238 to 7eb2bdf Compare August 8, 2023 06:24
This means that the new parser does interpret attached
double dashes as single words any more, but the old
parser never allowed that, so it is unproblematic
@eivindjahren eivindjahren merged commit 897aa3a into equinor:main Aug 8, 2023
39 of 42 checks passed
@eivindjahren eivindjahren deleted the fix_bug_with_double_comments branch August 8, 2023 07:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes:bug-fix Automatically categorise as bug fix in release notes
Projects
Archived in project
3 participants