`copilot-core`: record field `uTypeType` is unnecessary
Closed this issue · 6 comments
Description
The record field uTypeType
, part of Copilot.Core.Type.UType
, is never really used within the module or in any other part of Copilot or, as far as we know, by any user of Copilot. As a matter of fact, we cannot use it to obtain the value inside the UType
due to the existential it encloses. It also makes the test report lower coverage than they otherwise might, since the record access function is not used by the tests.
This field is not necessary parts of the interface to use Copilot, and not required by Copilot's requirements. Therefore, it would be suitable to remove that definitions from the module.
As per our internal policy of waiting 3 versions from deprecation until a public interface declaration can be removed, those definitions should be deprecated.
Type
- Bug: unused code included in the implementation.
Additional context
None.
Requester
- Ivan Perez
Method to check presence of bug
The field is mentioned in the record definition, but making it a non-record datatype, or not exporting that field in the module exports, does not change the behavior and does not otherwise affect Copilot.
The code compiles without errors and behaves in the same way.
Expected result
The following elements are deprecated:
Copilot.Core.Type.UType.uTypeType
Desired result
The following elements are deprecated:
Copilot.Core.Type.UType.uTypeType
Proposed solution
Deprecate the following functions:
Copilot.Core.Type.UType.uTypeType
Further notes
None.
Change Manager: Confirmed that the issue exists.
Technical Lead: Confirmed that the issue should be addressed.
Technical Lead: Issue scheduled for fixing in Copilot 4.1.
Fix assigned to: @ivanperez-keera .
Implementor: Solution implemented, review requested.
Change Manager: Verified that:
- Solution is implemented:
- The code proposed compiles and passes all tests. Details:
Build log: https://app.travis-ci.com/github/Copilot-Language/copilot/builds/272931486 - The solution proposed produces the expected result. Details:
The following Dockerfile tries to import and useuTypeType
, printing "Success" if the import is deprecated, and "Failure" otherwise:Command (substitute variables based on new path after merge):ENV DEBIAN_FRONTEND=noninteractive RUN apt-get update RUN apt-get install --yes libz-dev RUN apt-get install --yes git RUN apt-get install --yes wget RUN mkdir -p $HOME/.ghcup/bin RUN wget https://downloads.haskell.org/~ghcup/0.1.19.2/x86_64-linux-ghcup-0.1.19.2 -O $HOME/.ghcup/bin/ghcup RUN chmod a+x $HOME/.ghcup/bin/ghcup ENV PATH=$PATH:/root/.ghcup/bin/ ENV PATH=$PATH:/root/.cabal/bin/ RUN apt-get install --yes curl RUN apt-get install --yes gcc g++ make libgmp3-dev RUN apt-get install --yes pkg-config SHELL ["/bin/bash", "-c"] RUN ghcup install ghc 9.8.2 RUN ghcup install cabal 3.2 RUN ghcup set ghc 9.8.2 RUN cabal update SHELL ["/bin/bash", "-c"] CMD git clone $REPO && cd $NAME && git checkout $COMMIT && cd .. \ && cabal v1-sandbox init \ && cabal v1-install $NAME/copilot-core/ \ && ! cabal v1-exec -- runhaskell -XGADTs -Wall -Werror <<< 'import Copilot.Core.Type(UType(UType, uTypeType), Type(Int8)); main :: IO (); main = case UType Int8 of; (UType { uTypeType = Int8}) -> putStrLn "Failure"' \ && echo "Success"
$ docker run -e "REPO=https://github.com/ivanperez-keera/copilot" -e "NAME=copilot" -e "COMMIT=2a476637e44df5b1c125fa5c270c300479e0df72" -it copilot-verify-484
- The code proposed compiles and passes all tests. Details:
- Implementation is documented. Details:
No need for additional documentation (deprecation). - Change history is clear.
- Commit messages are clear.
- Changelogs are updated.
- Examples are updated. Details:
No updates needed (deprecation). - Required version bumps are evaluated. Details:
Major bump required. Public element being deprecated is an API-breaking change.
Change Manager: Implementation ready to be merged.