Skip to content

Final changes for glvis-4.2 #227

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.

Sign up for GitHub

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

Jump to bottom
Merged
tzanio merged 17 commits into from
May 23, 2022
Merged

Final changes for glvis-4.2 #227

tzanio merged 17 commits into from
May 23, 2022

Conversation

@tzanio
Copy link
Member

@tzanio tzanio commented Apr 13, 2022 •
edited
Loading

This will be the last PR to be merged before the glvis-4.2 release.

Release date

April 21st, 2022 (Thursday)
May 23th, 2022 (Monday)

TODO

After the release

Postponed

@tzanio tzanio added this to the glvis-4.2 milestone Apr 13, 2022
@tzanio tzanio added the release label Apr 13, 2022
@tzanio tzanio mentioned this pull request Apr 13, 2022
Merged
3 tasks
@tzanio tzanio self-assigned this Apr 13, 2022
@termi-official
Copy link
Contributor

termi-official commented Apr 14, 2022

Hi @tzanio , thanks for the consideration, but I think 1d elements needs more time until it is really ready to merge. The PR is somewhat functional at this point, but I think something went wrong in the way how I implemented the visualization.

@tzanio
Copy link
Member Author

tzanio commented Apr 14, 2022

Hi @tzanio , thanks for the consideration, but I think 1d elements needs more time until it is really ready to merge. The PR is somewhat functional at this point, but I think something went wrong in the way how I implemented the visualization.

Thanks @termi-official, I just wanted to check.

@tzanio
Copy link
Member Author

tzanio commented May 18, 2022

I added the Markdown version of the README in this branch, see https://github.com/GLVis/glvis/blob/glvis-4.2-dev/README.md. Let me know if you have any suggestions for improvements.

ping: @v-dobrev and @pazner

@v-dobrev
Copy link
Member

v-dobrev commented May 18, 2022

I added the Markdown version of the README in this branch, see https://github.com/GLVis/glvis/blob/glvis-4.2-dev/README.md. Let me know if you have any suggestions for improvements.

ping: @v-dobrev and @pazner

It may be helpful to add a reference to the keys/mouse functions at the beginning (maybe highlighted to draw attention) -- many people may not scroll down to see the keys/mouse sections. A good place is probably just after "For building instructions, see INSTALL."

@tzanio
Copy link
Member Author

tzanio commented May 19, 2022

It may be helpful to add a reference to the keys/mouse functions at the beginning (maybe highlighted to draw attention) -- many people may not scroll down to see the keys/mouse sections. A good place is probably just after "For building instructions, see INSTALL."

How is this: https://github.com/GLVis/glvis/blob/glvis-4.2-dev/README.md

@tzanio tzanio requested a review from tomstitt May 19, 2022 22:23
@tzanio
Copy link
Member Author

tzanio commented May 19, 2022

This is now a release candidate for glvis-4.2, please review and speak up if you'd like to see any changes.

@tzanio
Copy link
Member Author

tzanio commented May 23, 2022

Can I get some reviews/approvals for this please?

v-dobrev
v-dobrev reviewed May 23, 2022
View reviewed changes
CONTRIBUTING.md
Comment on lines 291 to 294
- [ ] Update the GLVis version in the following files:
- [ ] `CHANGELOG`
- [ ] `makefile`
- [ ] `CMakeLists.txt`
Copy link
Member

@v-dobrev v-dobrev May 23, 2022

Choose a reason for hiding this comment

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

Why remove these?

Copy link
Member Author

@tzanio tzanio May 23, 2022

Choose a reason for hiding this comment

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

There is no version actually in makefile or CMakeLists.txt

Copy link
Member

@v-dobrev v-dobrev May 23, 2022

Choose a reason for hiding this comment

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

And CHANGELOG?

Copy link
Member Author

@tzanio tzanio May 23, 2022

Choose a reason for hiding this comment

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

I thought that was obvious, but I can bring that one back if you prefer

Copy link
Member

@v-dobrev v-dobrev May 23, 2022

Choose a reason for hiding this comment

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

It's better to have this list -- I see version numbers in other places that we should add: e.g. share/Info.cmake.plist.in and share/Info.plist. There maybe other places too.

Copy link
Member Author

@tzanio tzanio May 23, 2022

Choose a reason for hiding this comment

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

ok

Copy link
Member

@v-dobrev v-dobrev May 23, 2022

Choose a reason for hiding this comment

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

Another file that has version: vcpkg.json.

Copy link
Member Author

@tzanio tzanio May 23, 2022

Choose a reason for hiding this comment

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

Done in 17e2a3c

v-dobrev
v-dobrev reviewed May 23, 2022
View reviewed changes
CHANGELOG Outdated

- Added the option to cut a portion of the interiors of 3D faces to expose more
of the mesh. Useful as an alternative to transparency. See keys Ctrl+F3/F4.
Version 4.2 released on May 20, 2022
Copy link
Member

@v-dobrev v-dobrev May 23, 2022

Choose a reason for hiding this comment

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

Update date?

Copy link
Member Author

@tzanio tzanio May 23, 2022

Choose a reason for hiding this comment

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

Done in 17e2a3c

v-dobrev
v-dobrev approved these changes May 23, 2022
View reviewed changes
Copy link
Member

@v-dobrev v-dobrev left a comment

Choose a reason for hiding this comment

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

A few minor issues as mentioned above.

Other than that, looks good!

publixsubfan
publixsubfan approved these changes May 23, 2022
View reviewed changes
Copy link
Contributor

@publixsubfan publixsubfan left a comment

Choose a reason for hiding this comment

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

LGTM

README.md

https://glvis.org

[GLVis](https://glvis.org) is an OpenGL tool for visualization of finite element
Copy link
Contributor

@publixsubfan publixsubfan May 23, 2022

Choose a reason for hiding this comment

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

Now that we have a Markdown file, we could consider adding some repo badges here (https://github.com/badges/shields).

Copy link
Member Author

@tzanio tzanio May 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Something like this?

                       GLVis visualization tool

                 _/_/_/  _/      _/      _/  _/
              _/        _/      _/      _/        _/_/_/
             _/  _/_/  _/      _/      _/  _/  _/_/
            _/    _/  _/        _/  _/    _/      _/_/
             _/_/_/  _/_/_/_/    _/      _/  _/_/_/

                         https://glvis.org

Release Build License Doxygen License License

@tzanio tzanio merged commit 386747f into master May 23, 2022
@tzanio tzanio deleted the glvis-4.2-dev branch May 23, 2022 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@v-dobrev v-dobrev v-dobrev approved these changes

@publixsubfan publixsubfan publixsubfan approved these changes

@mlstowell mlstowell Awaiting requested review from mlstowell

@tomstitt tomstitt Awaiting requested review from tomstitt

Assignees

@tzanio tzanio

Labels

release

Projects

None yet

Milestone

glvis-4.2

Development

Successfully merging this pull request may close these issues.

5 participants

@tzanio @termi-official @v-dobrev @publixsubfan