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: Implementation for the new mapping #1895

Merged
merged 1 commit into from
Jun 9, 2023

Conversation

Nurkambay
Copy link
Contributor

@Nurkambay Nurkambay commented May 24, 2023

This refactoring will fix the following issue: #1854

  1. Pay attention to the org.eclipse.lsp.cobol.common.mapping package.
    It contains a new implementation for the mapping. Other changes are just changes due to interface changes.

  2. Also there are some changes with tests. Some tests were updated due to better mapping and a few tests were disabled because of limitations in the use cases test suit. server/engine/src/test/java/org/eclipse/lsp/cobol/usecases

@Nurkambay Nurkambay marked this pull request as draft May 24, 2023 12:00
@Nurkambay Nurkambay force-pushed the mapping2-apply branch 4 times, most recently from aef2711 to f1f77bc Compare May 29, 2023 17:34
@Nurkambay Nurkambay marked this pull request as ready for review May 29, 2023 17:35
@Nurkambay Nurkambay added the enhancement Improvments of existing code label May 30, 2023
@Nurkambay Nurkambay added this to the 2.0.1 milestone May 30, 2023
@Nurkambay Nurkambay linked an issue May 30, 2023 that may be closed by this pull request
@Nurkambay Nurkambay force-pushed the mapping2-apply branch 6 times, most recently from 30a7d0e to c167eac Compare June 2, 2023 09:30
@VitGottwald
Copy link
Contributor

I download the artefacts (vsix-cobol-language-support-darwin-x64.zip, vsix-idms-dialect.zip, vsix-daco-dialect.zip) and tested on internal repo. It fails on every DaCo program. Will DM you on slack.

Copy link
Contributor

@VitGottwald VitGottwald left a comment

Choose a reason for hiding this comment

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

Since these changes affect the core functionality and some tests were disabled, please run the internal test suite and do a manual smoke test on the internal repo before proceeding.

@Nurkambay
Copy link
Contributor Author

Since these changes affect the core functionality and some tests were disabled, please run the internal test suite and do a manual smoke test on the internal repo before proceeding.

Thanks, I have added a use case test for it. Should be covered by unit tests now

@Nurkambay Nurkambay force-pushed the mapping2-apply branch 3 times, most recently from ba4fa99 to 9e92eb3 Compare June 7, 2023 12:32
@VitGottwald
Copy link
Contributor

Now both IDMS and DaCo code gets properly analysed. But now I am getting a random dance of errors showing up and disappearing when enter code. Maybe worth running the typing test?

@Nurkambay
Copy link
Contributor Author

Looks like it is something broken in the processing itself, i will check it in dev

@Nurkambay
Copy link
Contributor Author

Now both IDMS and DaCo code gets properly analysed. But now I am getting a random dance of errors showing up and disappearing when enter code. Maybe worth running the typing test?

I have checked it. It is completely the same functionality as we have on development. Looks like when we have errors we have faster processing then expected and that's why we can see these errors. And we have the same in the development branch.

Copy link
Contributor

@ap891843 ap891843 left a comment

Choose a reason for hiding this comment

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

👍

@Nurkambay Nurkambay merged commit cae08b9 into eclipse-che4z:development Jun 9, 2023
9 checks passed
@Nurkambay Nurkambay deleted the mapping2-apply branch June 9, 2023 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvments of existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Copy replacing doesn't work sometimes
4 participants