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

Add TlmViewerConfig spec and fix to_save #674

Merged
merged 2 commits into from Dec 1, 2017

Conversation

Projects
None yet
3 participants
@jasonatball
Collaborator

jasonatball commented Nov 30, 2017

No description provided.

@jasonatball jasonatball requested a review from ryanatball Nov 30, 2017

file.puts "NEW_COLUMN"
file.puts ''
end
file.puts "NEW_COLUMN\n\n" if column_index != 0

This comment has been minimized.

@ryanatball

ryanatball Nov 30, 2017

Member

The new lines caused by this are not equivalent.

@ryanatball

ryanatball Nov 30, 2017

Member

The new lines caused by this are not equivalent.

This comment has been minimized.

@jasonatball

jasonatball Nov 30, 2017

Collaborator

The old code didn't work with GROUP and GROUP_SCREEN so yes it is different.

@jasonatball

jasonatball Nov 30, 2017

Collaborator

The old code didn't work with GROUP and GROUP_SCREEN so yes it is different.

This comment has been minimized.

@ryanatball

ryanatball Nov 30, 2017

Member

I was referring to the newlines being different. Before it put one before and after the NEW_COLUMN

@ryanatball

ryanatball Nov 30, 2017

Member

I was referring to the newlines being different. Before it put one before and after the NEW_COLUMN

This comment has been minimized.

@jasonatball

jasonatball Dec 1, 2017

Collaborator

I think the newline at 244 takes care of newlines before. If you manually add a puts config on line 390 in the spec you'll see how it looks.

@jasonatball

jasonatball Dec 1, 2017

Collaborator

I think the newline at 244 takes care of newlines before. If you manually add a puts config on line 390 in the spec you'll see how it looks.

@ryanatball

This comment has been minimized.

Show comment
Hide comment
@ryanatball

ryanatball Nov 30, 2017

Member

Also new specs are failing

Member

ryanatball commented Nov 30, 2017

Also new specs are failing

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Nov 30, 2017

Codecov Report

Merging #674 into master will decrease coverage by 0.2%.
The diff coverage is 94.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #674      +/-   ##
==========================================
- Coverage   83.52%   83.32%   -0.21%     
==========================================
  Files         153      158       +5     
  Lines       14364    15392    +1028     
==========================================
+ Hits        11998    12825     +827     
- Misses       2366     2567     +201
Impacted Files Coverage Δ
lib/cosmos/tools/tlm_viewer/tlm_viewer_config.rb 92.14% <94.44%> (+73.86%) ⬆️
lib/cosmos/packet_logs/packet_log_writer.rb 92.21% <0%> (-1.2%) ⬇️
lib/cosmos/packets/parsers/xtce_parser.rb 62.05% <0%> (ø)
lib/cosmos/gui/dialogs/details_dialog.rb 4.58% <0%> (ø)
lib/cosmos/tools/tlm_viewer/widgets/widget.rb 1.35% <0%> (ø)
lib/cosmos/gui/dialogs/tlm_details_dialog.rb 1.69% <0%> (ø)
...cosmos/tools/tlm_viewer/widgets/vertical_widget.rb 7.14% <0%> (ø)
lib/cosmos/config/config_parser.rb 90.23% <0%> (+0.93%) ⬆️
lib/cosmos/gui/qt.rb 36.79% <0%> (+1.5%) ⬆️
lib/cosmos/tools/launcher/launcher_config.rb 96.74% <0%> (+1.62%) ⬆️
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9be4e86...de7986c. Read the comment docs.

codecov-io commented Nov 30, 2017

Codecov Report

Merging #674 into master will decrease coverage by 0.2%.
The diff coverage is 94.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #674      +/-   ##
==========================================
- Coverage   83.52%   83.32%   -0.21%     
==========================================
  Files         153      158       +5     
  Lines       14364    15392    +1028     
==========================================
+ Hits        11998    12825     +827     
- Misses       2366     2567     +201
Impacted Files Coverage Δ
lib/cosmos/tools/tlm_viewer/tlm_viewer_config.rb 92.14% <94.44%> (+73.86%) ⬆️
lib/cosmos/packet_logs/packet_log_writer.rb 92.21% <0%> (-1.2%) ⬇️
lib/cosmos/packets/parsers/xtce_parser.rb 62.05% <0%> (ø)
lib/cosmos/gui/dialogs/details_dialog.rb 4.58% <0%> (ø)
lib/cosmos/tools/tlm_viewer/widgets/widget.rb 1.35% <0%> (ø)
lib/cosmos/gui/dialogs/tlm_details_dialog.rb 1.69% <0%> (ø)
...cosmos/tools/tlm_viewer/widgets/vertical_widget.rb 7.14% <0%> (ø)
lib/cosmos/config/config_parser.rb 90.23% <0%> (+0.93%) ⬆️
lib/cosmos/gui/qt.rb 36.79% <0%> (+1.5%) ⬆️
lib/cosmos/tools/launcher/launcher_config.rb 96.74% <0%> (+1.62%) ⬆️
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9be4e86...de7986c. Read the comment docs.

@ryanatball

This comment has been minimized.

Show comment
Hide comment
@ryanatball

ryanatball Dec 1, 2017

Member

👍

Member

ryanatball commented Dec 1, 2017

👍

@jasonatball jasonatball merged commit 2230093 into master Dec 1, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@jasonatball jasonatball deleted the tlm_viewer_config_spec branch Dec 1, 2017

@jasonatball

This comment has been minimized.

Show comment
Hide comment
@jasonatball

jasonatball Dec 1, 2017

Collaborator

@ryanatball I don't have an issue with this. Do I need one for release book keeping? @

Collaborator

jasonatball commented Dec 1, 2017

@ryanatball I don't have an issue with this. Do I need one for release book keeping? @

@ryanatball

This comment has been minimized.

Show comment
Hide comment
@ryanatball

ryanatball Dec 1, 2017

Member

I can reference the pull request in the release notes. Just have to remember to do that. I'll leave this notification in my inbox so I don't forget. Release should be Monday.

Member

ryanatball commented Dec 1, 2017

I can reference the pull request in the release notes. Just have to remember to do that. I'll leave this notification in my inbox so I don't forget. Release should be Monday.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment