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

Interface modifiers and other fixes #656

Merged
merged 4 commits into from
May 10, 2023
Merged

Interface modifiers and other fixes #656

merged 4 commits into from
May 10, 2023

Conversation

ryanmelt
Copy link
Member

closes #653
closes #627

@ryanmelt ryanmelt requested a review from jmthomas May 10, 2023 03:57
@codecov
Copy link

codecov bot commented May 10, 2023

Codecov Report

Patch coverage: 65.15% and project coverage change: -0.06 ⚠️

Comparison is base (bbad0b7) 74.43% compared to head (3b82ecd) 74.38%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #656      +/-   ##
==========================================
- Coverage   74.43%   74.38%   -0.06%     
==========================================
  Files         464      464              
  Lines       28577    28599      +22     
  Branches      575      575              
==========================================
+ Hits        21271    21273       +2     
- Misses       7217     7237      +20     
  Partials       89       89              
Flag Coverage Δ
ruby-api 51.40% <ø> (+0.01%) ⬆️
ruby-backend 78.47% <65.15%> (-0.08%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
openc3/lib/openc3/models/interface_model.rb 82.25% <44.44%> (-6.98%) ⬇️
openc3/lib/openc3/packets/parsers/xtce_parser.rb 93.28% <50.00%> (-0.23%) ⬇️
openc3/lib/openc3/script/api_shared.rb 85.25% <91.66%> (ø)
openc3/lib/openc3/models/microservice_model.rb 91.73% <100.00%> (ø)
openc3/lib/openc3/utilities/ruby_lex_utils.rb 96.27% <100.00%> (+0.22%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

else
yield line, true, inside_begin, lex.exp_line_no
end
yield line, true, inside_begin, lex.exp_line_no
Copy link
Member

Choose a reason for hiding this comment

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

What were you trying to accomplish with these two blocks that got removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

They handled cases that now appear to be handled by paying attention to ltype.

@@ -699,6 +699,8 @@ def set_min_max_default(item, type, data_type)
# Strip quotes from strings
if type.initialValue[0] == '"' && type.initialValue[-1] == '"'
item.default = type.initialValue[1..-2]
else
item.default = type.initialValue
Copy link
Member

Choose a reason for hiding this comment

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

Are quote optional with XTCE?

Copy link
Member Author

Choose a reason for hiding this comment

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

Apparently. Someone ran into this case in COSMOS 4.

@@ -65,7 +65,7 @@ def _generate_auth

def _request(*method_params, **kw_params)
kw_params[:scope] = $openc3_scope unless kw_params[:scope]
kw_params[:json] => true unless kw_params[:json]
kw_params[:json] = true unless kw_params[:json]
Copy link
Member

Choose a reason for hiding this comment

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

This previously looks like a syntax error. Not even sure what this does. Did you look for other examples?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was a bug for sure. Setting the json flag adds the appropriate content-type header.

@ryanmelt ryanmelt merged commit 04e1995 into main May 10, 2023
@ryanmelt ryanmelt deleted the interface_modifiers branch May 10, 2023 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants