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

Exempt enhancements #191

Merged
merged 7 commits into from
Aug 5, 2024
Merged

Exempt enhancements #191

merged 7 commits into from
Aug 5, 2024

Conversation

fabio-looker
Copy link
Contributor

No description provided.

@fabio-looker fabio-looker merged commit 27fddaf into master Aug 5, 2024
3 checks passed
@mariana-s-fernandes
Copy link

hi @fabio-looker I believe this PR has broken my linting. All views for which I've added exemption are being caught by LAMS.

They are written as such:

include: "/_base/myproject/myview.view.lkml"

view: +myview {

  #LAMS
  #rule_exemptions: {F4: "2023-10-19 - This model will be probably deprecated"}

  ...

Should I change the formatting of the exemption?

@fabio-looker
Copy link
Contributor Author

fabio-looker commented Aug 5, 2024

Hi @mariana-s-fernandes - No, thiis existing format should still be supported, and there are many tests for it in the test suite...

I'll try to reproduce your issue, but if I can't, I may ask you for help pinpointing the issue.

In the meantime, are you able to continue using the prior LAMS version?

@mariana-s-fernandes
Copy link

I've reverted to 3.2.0 and it's all good. Yes, I can try help debugging. This is a refinement, not sure if that could be the cause of issue. I was getting a W1 error with that format.

@fabio-looker
Copy link
Contributor Author

@mariana-s-fernandes ok, that helps a lot, I thought originally you meant that the exemption was not working and causing, for example, F4 to error.

As for W1, this makes sense, that rule has no tests for conditional comments. I will fix this and make sure to add some tests for that going forward!

#193

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants