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

Feature/task provider for compile #1496

Draft
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

ap891843
Copy link
Contributor

No description provided.

@ap891843 ap891843 force-pushed the feature/taskProviderForCompile1 branch from 1680d0f to 3b799b9 Compare July 29, 2022 11:32
@VitGottwald
Copy link
Contributor

When testing with invalid APIML token, this is what the terminal looks like:
image

@VitGottwald
Copy link
Contributor

Nicely coloured output. Thank you!
image

A few comments:

  • The jobname column is redundant. It is the same in all rows. Instead the green message Job completed. ... could say Job completed, return code: CC 0000
  • The first green line is not needed, please remove it
  • The columns of the table are not aligned (spool-id should be aligned to the right, others to the left)
  • Please, swap the
  • Instead of the column, please use
  • Please, use the following column order: Spool-id, DDname, Stepname, Procstep
  • SYSPRINT at ... should be changed to Listing stored at ... and come before the list of errors.

@VitGottwald
Copy link
Contributor

After doing some more usability testing I believe that we should drop the "saveListing" and "listingLocation" configuration properties. Instead we should use vscode.ExtensionContext.storagePath folder to store the listings.

If the folder does not exist, we should create it on activation and create a listings subfolder to store the listings. If it exists we should clean (delete) the listings subfolder. The retrieved listings should then be stored in the listings subfolder a ${hostname}/${jobid}.txt files during a session. They will stay there to VScode reload (or extension de-activation followed by activation). For the deletion, please, use deleteFile vscode api. Unlike 'node' APIs it is guaranteed to work in all supported environements where vscode runs.

The extension context is provided to the extension by vscode as an argument on activate.

@VitGottwald
Copy link
Contributor

Please, add listFiles: boolean task configuration property with default true. If false, the table of DDs would not be printed to the output.

@ap891843 ap891843 marked this pull request as draft August 1, 2022 09:49
@ap891843
Copy link
Contributor Author

ap891843 commented Aug 1, 2022

Nicely coloured output. Thank you! image

A few comments:

  • The jobname column is redundant. It is the same in all rows. Instead the green message Job completed. ... could say Job completed, return code: CC 0000
  • The first green line is not needed, please remove it
  • The columns of the table are not aligned (spool-id should be aligned to the right, others to the left)
  • Please, swap the
  • Instead of the column, please use
  • Please, use the following column order: Spool-id, DDname, Stepname, Procstep
  • SYSPRINT at ... should be changed to Listing stored at ... and come before the list of errors.

Updated as below
image

@ap891843
Copy link
Contributor Author

ap891843 commented Aug 1, 2022

When testing with invalid APIML token, this is what the terminal looks like: image

Updated, tested with other error
image

@ap891843
Copy link
Contributor Author

ap891843 commented Aug 2, 2022

After doing some more usability testing I believe that we should drop the "saveListing" and "listingLocation" configuration properties. Instead we should use vscode.ExtensionContext.storagePath folder to store the listings.

If the folder does not exist, we should create it on activation and create a listings subfolder to store the listings. If it exists we should clean (delete) the listings subfolder. The retrieved listings should then be stored in the listings subfolder a ${hostname}/${jobid}.txt files during a session. They will stay there to VScode reload (or extension de-activation followed by activation). For the deletion, please, use deleteFile vscode api. Unlike 'node' APIs it is guaranteed to work in all supported environements where vscode runs.

The extension context is provided to the extension by vscode as an argument on activate.

Hello Vit, I have a few doubts about this approach.

  1. Planned to use storageUri as storagePath is deprecated
  2. These attributes are read-only and can be undefined when we open a file (not a workspace or a folder). COBOL LS works for a single file as well. So, ideally, the compile should also work.

And would need some refactoring for this. I plan to do it in a separate PR.

@ap891843 ap891843 force-pushed the feature/taskProviderForCompile1 branch from 21673b7 to 4453b7f Compare August 2, 2022 09:39
@ap891843
Copy link
Contributor Author

ap891843 commented Aug 2, 2022

ble of DDs would not

updated

@ap891843 ap891843 force-pushed the feature/taskProviderForCompile1 branch from 4453b7f to 2fc7bb7 Compare August 2, 2022 09:46
@ap891843 ap891843 marked this pull request as ready for review August 2, 2022 09:47
@ap891843 ap891843 force-pushed the feature/taskProviderForCompile1 branch from 2fc7bb7 to 4657390 Compare August 2, 2022 09:48
@ap891843 ap891843 force-pushed the feature/taskProviderForCompile1 branch 3 times, most recently from 30b4cc8 to d8029b1 Compare August 3, 2022 14:51
@ap891843 ap891843 marked this pull request as draft August 12, 2022 15:50
@ap891843 ap891843 force-pushed the feature/taskProviderForCompile1 branch 3 times, most recently from 5d88984 to 28331f4 Compare August 18, 2022 14:31
@ap891843 ap891843 force-pushed the feature/taskProviderForCompile1 branch 3 times, most recently from 1e58157 to 50ea33b Compare August 31, 2022 14:09
@ap891843 ap891843 changed the title Feature/task provider for compile1 Feature/task provider for compile Aug 31, 2022
Add tasks for cobol compilation

Co-authored-by: Vit Gottwald <vit.gottwald@gmail.com>
Signed-off-by: ap891843 <aman.prashant@broadcom.com>
@ap891843 ap891843 force-pushed the feature/taskProviderForCompile1 branch from 50ea33b to 23629a8 Compare December 30, 2022 13:12
@ishche
Copy link
Contributor

ishche commented Oct 17, 2023

What's up with this RP?
Do you need help to finish it?

@ap891843
Copy link
Contributor Author

What's up with this RP? Do you need help to finish it?

Yes, I would need some help here.
I am waiting for @VitGottwald to define the acceptance criteria.

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.

None yet

3 participants