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

export ValidationResults #192

Merged

Conversation

mx1up
Copy link

@mx1up mx1up commented Jul 24, 2024

Ultimate problem:

ValidationResults can not be used outside package

How it was fixed:

export the class

fixes #175

@mx1up mx1up requested a review from a team as a code owner July 24, 2024 15:52
@aviary-wf
Copy link

Security Insights

No security relevant content was detected by automated scans.

Action Items

  • Review PR for security impact; comment "security review required" if needed or unsure
  • Verify aviary.yaml coverage of security relevant code

Questions or Comments? Reach out on Slack: #support-infosec.

Copy link

@michaelcarter-wf michaelcarter-wf left a comment

Choose a reason for hiding this comment

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

+1

@joecotton-wk
Copy link

@mx1up Thanks for adding this! Would you do us a favor and revert 5.1.8 back to 5.1.7? That will allow our CI system to bump the version automatically. Once that's done, we'll get this one merged 👍

@mx1up
Copy link
Author

mx1up commented Jul 24, 2024

@mx1up Thanks for adding this! Would you do us a favor and revert 5.1.8 back to 5.1.7? That will allow our CI system to bump the version automatically. Once that's done, we'll get this one merged 👍

sure, i reverted the commit bumping the version, which also reverts the changelog as I guess it's also no longer in use..

@mx1up mx1up requested a review from michaelcarter-wf July 24, 2024 18:06
joecotton-wk
joecotton-wk previously approved these changes Jul 24, 2024
Copy link

@joecotton-wk joecotton-wk left a comment

Choose a reason for hiding this comment

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

+1

@joecotton-wk
Copy link

@mx1up Now format is failing 😔 Would you mind fixing? Thank you!

Once that's done (and barring no other problems), we'll get this merged for you.

@joecotton-wk
Copy link

joecotton-wk commented Jul 24, 2024

@mx1up Come to think of it, I have no idea why format is failing 🤔 It's the same as it was before you reverted the version bump.

I also pulled down your branch and ran format on it, and it came back clean. So I'm confused why it's failing in CI.

Let me see if it's our CI at fault.

@mx1up
Copy link
Author

mx1up commented Jul 24, 2024

@joecotton-wk were those checks run on the last 2 PRs? I don't see them in the PR activity log. My guess is:

that could explain it..

@mx1up
Copy link
Author

mx1up commented Jul 24, 2024

either way, it needs to be fixed , so I updated the fork with latest master and reformatted the file

@mx1up mx1up requested a review from joecotton-wk July 24, 2024 19:49
Copy link

@joecotton-wk joecotton-wk left a comment

Choose a reason for hiding this comment

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

+1

@joecotton-wk
Copy link

@Workiva/release-management-p

Copy link

@rmconsole-wf rmconsole-wf left a comment

Choose a reason for hiding this comment

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

+1 from RM

@rm-astro-wf rm-astro-wf merged commit a9dc04b into Workiva:master Jul 24, 2024
4 checks passed
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.

ValidationResults not exported
6 participants