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

Improve boolean and enum pv managment in pvtable and open Web Page Management #3246

Merged
merged 3 commits into from
Jan 30, 2025

Conversation

katysaintin
Copy link
Contributor

@katysaintin katysaintin commented Jan 29, 2025

Display Boolean value in PVTable instead of VBoolean.toString and use a ComboBox [0,1] to set Boolean Value
Fix some issues on VEnum management

And improve Open Web Page for secured https page in WebBrowser

@katysaintin katysaintin changed the title Improve boolean and enum pv managment in pvtable Improve boolean and enum pv managment in pvtable and open Web Page Management Jan 30, 2025
@katysaintin
Copy link
Contributor Author

Hello @kasemir & @shroffk ,

Do you prefer 2 separates PR or is it OK like that ?
If you are OK, Il will merge my PR and close the PR #2991 of @Mathis-Hu .

Thank you again.
Best regards,

Katy

@kasemir
Copy link
Collaborator

kasemir commented Jan 30, 2025

If you are OK, Il will merge my PR and close the PR #2991 of @Mathis-Hu .

Yes, do that

@shroffk
Copy link
Member

shroffk commented Jan 30, 2025

Ideally 2 PR's would have been best but we can resolve this for now.

I believe resource.openConnection(); is more efficient than opening an inputstream to the resource.

Can we remove a bunch of the comments which are not needed
like // system.... // print error etc...

@katysaintin
Copy link
Contributor Author

Ideally 2 PR's would have been best but we can resolve this for now.

Yes I understand, I will take care next time to separate PR.

Can we remove a bunch of the comments which are not needed
like // system.... // print error etc...

Yes it is removed and replace by logger

@shroffk
Copy link
Member

shroffk commented Jan 30, 2025

Looks good to me

@kasemir sorry I gave contradictory guidance (I missed your comment).

@kasemir
Copy link
Collaborator

kasemir commented Jan 30, 2025

@shroffk No worries

@katysaintin katysaintin merged commit 666d9ad into ControlSystemStudio:master Jan 30, 2025
1 check passed
@katysaintin
Copy link
Contributor Author

@kasemir & @shroffk
Damned , the build failed because of this deprecation
https://github.blog/changelog/2024-04-16-deprecation-notice-v3-of-the-artifact-actions/
The date of the update is today January 30th, 2025 , so I'm so unlucky.
I will found the solution ASAP to fix that .

Katy

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.

3 participants