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

Add regex-tdfa to preloaded packages #52

Merged
merged 1 commit into from
Jul 15, 2022
Merged

Conversation

asarkar
Copy link
Contributor

@asarkar asarkar commented Jul 7, 2022

regex-tdfa is a fairly common package, and other tracks do allow for use of regex, so it's not philosophically banned either. It is very useful for several exercises.

regex-tdfa is a fairly common package, and other tracks do allow for use of regex, so it's not philosophically banned either. It is very useful for several exercises.
@asarkar asarkar requested a review from a team as a code owner July 7, 2022 09:14
@asarkar
Copy link
Contributor Author

asarkar commented Jul 7, 2022

Related to exercism/haskell#1045

@ErikSchierboom
Copy link
Member

@petertseng This look good to you?

@petertseng
Copy link
Member

yes, I confirm there is no ban on regex. To be clear: I explicitly express my approval of this PR. The only reason I'm not Approving in the GitHub sense of the word is that I consider myself to have no power here (even though I have write permissions in this repository, the previous sentence is meant to express the social factor governed by self-regulation, rather than the technical factor governed by permissions/technology)

Copy link
Member

@petertseng petertseng left a comment

Choose a reason for hiding this comment

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

I changed my mind and there are a variety of reasons that I should Approve in the GitHub sense of the word (the reasons boil down to visibility and ease of understanding my viewpoint at a glance when reviewing history). My previous statements do of course still apply!

@asarkar
Copy link
Contributor Author

asarkar commented Jul 14, 2022

I still see one pending reviewer that looks like a bot. Do I need to do something to move this forward?

@petertseng
Copy link
Member

From my point of view, we have everything we need from you at this moment, but rest assured that we stand ready to let you know if that happens to change.

However, I would be far more hesitant to be discourteous to the organisations admins by calling them bots.

@petertseng
Copy link
Member

I owe an explanation here - the reason I have not merged this is I believe my merge has no power. Even if I merged it, I cannot update the test runner, so it's better for me to leave the merging to someone who has the ability to make that happen.

If there needs to be a process improvement so that a track maintainer can do this, we could have a discussion on what changes would need to be made so that this is possible, but I've usually been taught to go with the principle of least privilege, and giving me the power to break something that I really should not be allowed to break is an uncomfortable prospect.

Copy link
Member

@ErikSchierboom ErikSchierboom left a comment

Choose a reason for hiding this comment

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

I'm happy to be the one making the decision on whether to merge this or not.

This looks good to me! Thanks @asarkar

@asarkar
Copy link
Contributor Author

asarkar commented Jul 16, 2022

Now that this PR is merged, can I start using this package, or do I have to wait for a release?

@ErikSchierboom
Copy link
Member

@asarkar The only thing you had to wait for was the deploy workflow, which usually finishes in 10-20 minutes after merging a PR: https://github.com/exercism/haskell-test-runner/actions/runs/2678146178 So, yes: you can use it.

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.

3 participants