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.

Sign up for GitHub

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

Jump to bottom

Add Rule Identifier to RuleSlot #83

Merged
merged 5 commits into from
Mar 22, 2022
Merged

Add Rule Identifier to RuleSlot #83

merged 5 commits into from
Mar 22, 2022

Conversation

DJManuel
Copy link
Contributor

@DJManuel DJManuel commented Mar 7, 2022

This PR adds the ruleIdentifier to the RuleSlot Props to cleraly identify the exact rule that is being used. Related to Issue #81

@DJManuel DJManuel mentioned this pull request Mar 7, 2022
Closed
@rtucek rtucek added documentation Improvements or additions to documentation enhancement New feature or request labels Mar 7, 2022
@rtucek
Copy link
Owner

rtucek commented Mar 7, 2022 •
edited
Loading

Please consider the following TODOs:

@DJManuel
Copy link
Contributor Author

DJManuel commented Mar 8, 2022

TS builds and interface has been fixed. In the Documentation some mention has been added for it.
I am sorry, but I am quite new with jest, so I hope the unit test implementation is fine how it is done.

rtucek
rtucek requested changes Mar 9, 2022
View reviewed changes
Copy link
Owner

@rtucek rtucek left a comment

Choose a reason for hiding this comment

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

Thx for your contribution.

I only found 2 minor things 😉

docs/styling.md Outdated Show resolved Hide resolved
tests/components/Component.vue Outdated Show resolved Hide resolved
@rtucek rtucek added the testing Change relates to software testing label Mar 9, 2022
@DJManuel
Copy link
Contributor Author

Those two minor things are corrected and should be ready to be merged. Thank you!

@DJManuel
Copy link
Contributor Author

The Failing CI Testing seems to be a permissions issue:
See here
compared to a working PR
I am not quite sure exactly, but I would guess some of those token items would need to be "write" in order to create a comment on this PR.

@rtucek rtucek linked an issue Mar 22, 2022 that may be closed by this pull request
RuleSlot exact rule identification #81
Closed
rtucek added a commit that referenced this pull request Mar 22, 2022
@rtucek
Remove code coverage upload in PRs
5ab9880 
As seen in #83, code coverage upload as a comment in the PR doesn't work if the PR is opened by
someone without direct write access to this repository.

This is due to a [limitation in Github
Actions](https://github.saobby.my.eu.orgmunity/t/github-actions-are-severely-limited-on-prs/18179#M9249).
@rtucek rtucek mentioned this pull request Mar 22, 2022
Merged
@rtucek rtucek changed the title have rule identifier available in RuleSlot Add Rule Identifier to RuleSlot Mar 22, 2022
@rtucek
Copy link
Owner

rtucek commented Mar 22, 2022

The test itself is fine and passes. The Github action pipeline fails because it can't upload the code coverage report (which is supposed to show up as a comment here). Apparently, I assumed this will "just work" regardless if I or someone else will open a PR. This is a fundamental problem with Github itself... I've removed the coverage report action as it will only work if I open the PR by myself :/

In any case - thank you very much for you contribution 🙏

@rtucek rtucek merged commit 9c5b75f into rtucek:main Mar 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rtucek rtucek rtucek approved these changes

Assignees
No one assigned
Labels
documentation Improvements or additions to documentation enhancement New feature or request testing Change relates to software testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

RuleSlot exact rule identification
2 participants
@DJManuel @rtucek