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

[Refactor] imports to use Relation.Nullary.Irrelevant #2676

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

jmougeot
Copy link
Contributor

@jmougeot jmougeot commented Mar 14, 2025

Update imports to replace references to Relation.Nullary with Relation.Nullary.Irrelevant where applicable, enhancing clarity and organization in the codebase. #2624

Copy link
Member

@Taneb Taneb left a comment

Choose a reason for hiding this comment

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

There's a new module, so there should be a changelog entry

@jmougeot
Copy link
Contributor Author

jmougeot commented Mar 14, 2025

I just added the changelog entry, but I’m not sure if I did it correctly.

@jamesmckinna
Copy link
Contributor

Also a good idea, for the sake of the GitHub history, to explicitly mention #2624 , which this now does over as a 'clean' PR.

Copy link
Contributor

@jamesmckinna jamesmckinna left a comment

Choose a reason for hiding this comment

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

Besides the nitpick, I am now convinced about the benefits of the refactoring, esp. wrt removing the imports with hiding (Irrelevant) directives.

CHANGELOG.md Outdated
@@ -120,6 +120,8 @@ New modules

* `Data.Sign.Show` to show a sign

* `Relation.Nullary.Irrelevant` defining the concept of irrelevance (h-proposition), where all elements of a type are equal.
Copy link
Contributor

Choose a reason for hiding this comment

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

While this is a new module, it really is a refactoring, so this isn't quite the right place to put this. I'm sure previous CHANGELOGs have such entries that can be used as a guide on how to do this.

@jmougeot jmougeot changed the title Refactor imports to use Relation.Nullary.Irrelevant [Refactor] imports to use Relation.Nullary.Irrelevant #2624 Mar 17, 2025
@jmougeot jmougeot changed the title [Refactor] imports to use Relation.Nullary.Irrelevant #2624 [Refactor] imports to use Relation.Nullary.Irrelevant Mar 17, 2025
Copy link
Contributor

@jamesmckinna jamesmckinna left a comment

Choose a reason for hiding this comment

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

Suggested tweak, but optional.

Co-authored-by: jamesmckinna <[email protected]>
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.

4 participants