Remove the concept of the 'current module' in Docs - #3506
Merged
Conversation
The concept of the current module is unnecessary (we aren't using it anywhere either in the compiler or in Pursuit) as well as potentially confusing; for example, what is the 'current module' when we are rendering re-exported declarations? Additionally, since the information is not used, it would be easy for it to become incorrect without anyone noticing. This commit refactors the Docs related code, removing the concept of the current module. Specifically, the following have been removed: - the SameModule constructor from the LinkLocation data type; wherever we previously would have used that, we can now use the LocalModule constructor, which encodes precisely the same information. - the 'current module' field from the constructors LocalModule and DepsModule of the data type LinkLocation, which was unused. - the 'currentModuleName' field of the HtmlRenderContext data type, which was also unused. I came across this refactoring opportunity while looking into #3504.
garyb
approved these changes
Jan 7, 2019
Contributor
Author
|
Thanks for the review! |
dariooddenino
pushed a commit
to dariooddenino/purescript
that referenced
this pull request
Jan 18, 2019
The concept of the current module is unnecessary (we aren't using it anywhere either in the compiler or in Pursuit) as well as potentially confusing; for example, what is the 'current module' when we are rendering re-exported declarations? Additionally, since the information is not used, it would be easy for it to become incorrect without anyone noticing. This commit refactors the Docs related code, removing the concept of the current module. Specifically, the following have been removed: - the SameModule constructor from the LinkLocation data type; wherever we previously would have used that, we can now use the LocalModule constructor, which encodes precisely the same information. - the 'current module' field from the constructors LocalModule and DepsModule of the data type LinkLocation, which was unused. - the 'currentModuleName' field of the HtmlRenderContext data type, which was also unused. I came across this refactoring opportunity while looking into purescript#3504.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The concept of the current module is unnecessary (we aren't using it
anywhere either in the compiler or in Pursuit) as well as potentially
confusing; for example, what is the 'current module' when we are
rendering re-exported declarations? Additionally, since the information
is not used, it would be easy for it to become incorrect without anyone
noticing.
This commit refactors the Docs related code, removing the concept of the
current module. Specifically, the following have been removed:
wherever we previously would have used that, we can now use the
LocalModule constructor, which encodes precisely the same information.
DepsModule of the data type LinkLocation, which was unused.
which was also unused.
I came across this refactoring opportunity while looking into #3504.