Skip to content

Feature/literal flag options#191

Open
redxeth wants to merge 9 commits intomasterfrom
feature/literal_flag_options
Open

Feature/literal flag options#191
redxeth wants to merge 9 commits intomasterfrom
feature/literal_flag_options

Conversation

@redxeth
Copy link
Member

@redxeth redxeth commented Apr 14, 2023

Added ability to do additonal custom changes to flag names-- in case user has particular requirements.

Flag name modifications:

  • Added ability to substitute the word "FAILED" for any custom word. Also abilty to change "PASSED" and "RAN" as part of flag name also.
  • Added ability to put the type of flag (fail/pass/ran) first instead of last in the name.

See environment/*_literals.rb files for examples.

redxeth added 6 commits March 31, 2023 12:57
when defining flag names, to support NXP flag naming requirements.  Old
flag naming is the default.
files.  User will have to handle as part of the flag ID.
Added option to remove flow id MD5 hash from flag name.
(Thao doing this work).
@redxeth
Copy link
Member Author

redxeth commented Apr 24, 2023

Any comments? Should I just merge it? Need to make a new release. @ginty @priyavadan @pderouen @chrisnappi

@pderouen
Copy link
Member

I've been holding my peace on this one. The changes suit our purposes. Thank you for this PR. From what I can tell the change maintains the previous behavior as the default. So, I think it's good to go. @priyavadan, feel free to weigh in.

The only update I would request is to document these new options in the guides page.

@info-rchitect
Copy link
Member

I've been holding my peace on this one. The changes suit our purposes. Thank you for this PR. From what I can tell the change maintains the previous behavior as the default. So, I think it's good to go. @priyavadan, feel free to weigh in.

The only update I would request is to document these new options in the guides page.

Agreed on the docs update

Copy link
Member

@priyavadan priyavadan left a comment

Choose a reason for hiding this comment

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

looks OK to me. I just verified internally that the default behavior stays the same.

I'll second the request to add more documentation around this feature in the guides though.

@redxeth
Copy link
Member Author

redxeth commented Apr 24, 2023

Yep-- I was expecting the doc thing from you guys. Can someone remind me how to update the docs again for something OrigenTesters specific? Is there a procedure for this somewhere?

First update docs in the plug-in-- then? Will Origen main guides pick up the change automatically?

@pderouen
Copy link
Member

Yep-- I was expecting the doc thing from you guys. Can someone remind me how to update the docs again for something OrigenTesters specific? Is there a procedure for this somewhere?

First update docs in the plug-in-- then? Will Origen main guides pick up the change automatically?

It's been a while. But, I believe you update the guides in templates/origen_guides/program of origen_testers. After it's merged, the website can be updated by running web compile -r from origen.

@redxeth
Copy link
Member Author

redxeth commented Apr 25, 2023

It's been a while. But, I believe you update the guides in templates/origen_guides/program of origen_testers. After it's merged, the website can be updated by running web compile -r from origen.

Yes web compile -r is to regen and publish a new website-- I forgot about the way they are shared-- so the web publish / deploy within the OrigenTesters plug-in will only update that folder on the main origen website looks like.

@redxeth
Copy link
Member Author

redxeth commented Apr 25, 2023

Ok so in the process of making some updates, I'm seeing lots of lint changes-- I assume that's what all these recent rubocop PRs are about?

So to confirm @priyavadan @pderouen @CodyBriscoe -- my options are either to confirm the rubocop recommendations don't break OrigenTesters or add the exceptions?

And all of this because of the rubocop upgrade needed for Ruby 3.1 ?

UGH-- this is why people are so loathe to make Origen updates on their own (I'm doing this for someone else)

@redxeth
Copy link
Member Author

redxeth commented Apr 25, 2023

If someone could help fix would appreciate it-- having to go look through multiple PRs-- If this breaks many plugins then we have a lot of 'splaining to do lucy'

@pderouen
Copy link
Member

Ok so in the process of making some updates, I'm seeing lots of lint changes-- I assume that's what all these recent rubocop PRs are about?

So to confirm @priyavadan @pderouen @CodyBriscoe -- my options are either to confirm the rubocop recommendations don't break OrigenTesters or add the exceptions?

And all of this because of the rubocop upgrade needed for Ruby 3.1 ?

UGH-- this is why people are so loathe to make Origen updates on their own (I'm doing this for someone else)

There are a few more options. I took my best shot at setting up the Rubocop rules so it wouldn't be too disruptive.

  1. If we think there are more rules that need to be turned off, that can be done.
  2. We could also have a custom ruleset for origen_testers.
  3. We could add a third "legacy" option to origen lint.
  4. We might be able to add an origen dependency to the Gemfile and restrict it to the last version before the Rubocop update. That would keep the regression checks running with the older version.

Since it's my group causing the need for this PR and me that made the Ruby 3 updates, I should probably help figure out the right solution.

@redxeth
Copy link
Member Author

redxeth commented Apr 26, 2023

Since it's my group causing the need for this PR and me that made the Ruby 3 updates, I should probably help figure out the right solution.

Thanks @pderouen ! We should as a group decide on how to handle 'breaking' changes done in Origen and all plug-ins. If there is a breaking change (and here I realize the code isn't broken but needs to be upgraded to match some upgrade in a plug-in).

Do poeple still use the discord server? I checked and see the latest post from last year.

Perhaps github issues? If something new we think may break others then file an issue with attention to the core team, etc., of what things need to be done. Or add a deprecate warning somehow.

Or if it has already broken something, then file an issue here - which I can do and add you @pderouen so that in case others encounter this they understand it's a known issue and how to resolve.

@pderouen
Copy link
Member

Since it's my group causing the need for this PR and me that made the Ruby 3 updates, I should probably help figure out the right solution.

Thanks @pderouen ! We should as a group decide on how to handle 'breaking' changes done in Origen and all plug-ins. If there is a breaking change (and here I realize the code isn't broken but needs to be upgraded to match some upgrade in a plug-in).

Do poeple still use the discord server? I checked and see the latest post from last year.

Perhaps github issues? If something new we think may break others then file an issue with attention to the core team, etc., of what things need to be done. Or add a deprecate warning somehow.

Or if it has already broken something, then file an issue here - which I can do and add you @pderouen so that in case others encounter this they understand it's a known issue and how to resolve.

You were on all of the issues and PR's for the Rubocop update. All of the discussions about it were had in the monthly global team calls. Agreed, there could also be a bulletin or something of that sort. Unfortunately, there just wasn't a way to stay current with Ruby and keep the ancient version of Rubocop that origen was previously locked with. We'll see if the ruleset can be further refined to make it less disruptive.

By the way there is still 1 open PR on origen that is needed for it to work with Ruby > 3.0.

@pderouen
Copy link
Member

I am working through the lint errors and will push an update to this PR soon. Some of the failures seem to be highlighting bugs. For those, I will just disable the check for that piece of code and file an issue rather than piling on to this PR.

@redxeth
Copy link
Member Author

redxeth commented Apr 27, 2023

You were on all of the issues and PR's for the Rubocop update. All of the discussions about it were had in the monthly global team calls.

Yeah sorry-- have a hard time keeping up sometimes with everything.

@redxeth
Copy link
Member Author

redxeth commented Apr 27, 2023

I am working through the lint errors and will push an update to this PR soon. Some of the failures seem to be highlighting bugs. For those, I will just disable the check for that piece of code and file an issue rather than piling on to this PR.

Wow thanks for your help!

@pderouen
Copy link
Member

I am working through the lint errors and will push an update to this PR soon. Some of the failures seem to be highlighting bugs. For those, I will just disable the check for that piece of code and file an issue rather than piling on to this PR.

Wow thanks for your help!

It's the least I could do. The lint fix is coming in just a minute. I apologize in advance for the huge number of additional diffs. But, since we already reviewed the content of the PR and approved it hopefully won't be an issue.


# Legacy option provided by OrigenTesters that permits override of a block enable method by passing
# an :or option with a true value
if (CONDITION_KEYS[method] == :if_enabled || CONDITION_KEYS[method] == :unless_enabled) && options[:or]
Copy link
Member

Choose a reason for hiding this comment

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

this fixes #192

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants