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.

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

Use edge as default browser and use modern help #1736 #1737

Merged
merged 2 commits into from
Jun 14, 2024
Merged

Use edge as default browser and use modern help #1736 #1737

merged 2 commits into from
Jun 14, 2024

Conversation

wimjongman
Copy link
Contributor

No description provided.

@wimjongman wimjongman added this to the 4.17 milestone Jun 14, 2024
@wimjongman wimjongman linked an issue Jun 14, 2024 that may be closed by this pull request
Use edge as default browser and use modern help #1736
Closed
@merks merks self-requested a review June 14, 2024 15:55
merks
merks requested changes Jun 14, 2024
View reviewed changes
Copy link
Contributor

@merks merks left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I'm really confused about which product is what. I can't stand all these confusing *.product files. 😱

In any case, you have a bunch of changes here that don't appear related to the title of the PR...

@wimjongman wimjongman force-pushed the Use_edge_as_default_browser_and_use_modern_help_#1736 branch from e00dd0d to 3f995c0 Compare June 14, 2024 16:01
@merks
Copy link
Contributor

merks commented Jun 14, 2024

If I look at the IDs correctly I think this is the one we ship isn't it?

image

@wimjongman
Copy link
Contributor Author

Yes, this is a mess. I think I got the correct one now.

@merks
Copy link
Contributor

merks commented Jun 14, 2024

Yes, I'm pretty sure:

image

Also in this report I just generated:

https://download.eclipse.org/oomph/archive/reports-extra/birt-4.16/download.eclipse.org/birt/updates/release/4.16.0/index/org.eclipse.birt.designer_4.16.0.v202406131946.html

All these *.product files are way too confusing...

@wimjongman
Copy link
Contributor Author

I updated it yesterday as well and I got the correct changes in the all-in-one

@wimjongman
Copy link
Contributor Author

bb16316

@merks
Copy link
Contributor

merks commented Jun 14, 2024

Yes, this is a mess. I think I got the correct one now.

This suggests that we put one product in the update site but zip up a different one for redistribution: one which is not actually in the update site. I really could literally just scream...

Also, multiple commits in a PR make me unhappy. These all end up in the history. And pushing branches to the main repo instead of too a fork also makes me unhappy:

image

It would be so much better to use a workflow like this one:

https://github.com/orgs/eclipse-simrel/discussions/3

In any case, it's very late which also makes me unhappy.

@wimjongman
Copy link
Contributor Author

Yes, this is a mess. I think I got the correct one now.

This suggests that we put one product in the update site but zip up a different one for redistribution: one which is not actually in the update site. I really could literally just scream...

LOL! Yes, I will work on that for the 4.17 release.

Also, multiple commits in a PR make me unhappy. These all end up in the history. And pushing branches to the main repo instead of too a fork also makes me unhappy:

image

It would be so much better to use a workflow like this one:

https://github.com/orgs/eclipse-simrel/discussions/3

We use exactly that workflow 🎉

In any case, it's very late which also makes me unhappy.

Late in the day? Or late to get this in 4.16? 4.16 has already sailed.

@merks
Copy link
Contributor

merks commented Jun 14, 2024

Late in the day. A glass of wine 🍷 improves the mood. We can make 4.17 better. After all, no one complained about it. 😜

@wimjongman
Copy link
Contributor Author

Late in the day. A glass of wine 🍷 improves the mood.

Cheers! You deserved it.

@wimjongman wimjongman merged commit af94371 into master Jun 14, 2024
3 checks passed
@wimjongman wimjongman deleted the Use_edge_as_default_browser_and_use_modern_help_#1736 branch June 14, 2024 17:14
@speckyspooky speckyspooky added the Enhancement Small change to improve the current supported functionality label Jun 16, 2024
wimjongman added a commit that referenced this pull request Sep 11, 2024
@wimjongman
The preview does not work anymore: #1901
3c462d4 
After switching the default browser to Edge in issues #1736 and #1737
the preview pane was broken because Edge does not like plain filenames.

Fixed:

1. Convert the plain file name to a file URL

This also fixed:

* Ask to save the editor when it is dirty and Preview is
requested. #1902
* 'New Preview Prototype' tab must be called Preview #1903
@wimjongman wimjongman mentioned this pull request Sep 11, 2024
Merged
wimjongman added a commit that referenced this pull request Sep 12, 2024
@wimjongman
The preview does not work anymore: #1901
037ccbf 
After switching the default browser to Edge in issues #1736 and #1737
the preview pane was broken because Edge does not like plain filenames.

Fixed:

1. Convert the plain file name to a file URL

This also fixed:

* Ask to save the editor when it is dirty and Preview is
requested. #1902
* 'New Preview Prototype' tab must be called Preview #1903
wimjongman added a commit that referenced this pull request Sep 12, 2024
@wimjongman
The preview does not work anymore: #1901 (#1904)
aa849c4 
After switching the default browser to Edge in issues #1736 and #1737
the preview pane was broken because Edge does not like plain filenames.

Fixed:

1. Convert the plain file name to a file URL

This also fixed:

* Ask to save the editor when it is dirty and Preview is
requested. #1902
* 'New Preview Prototype' tab must be called Preview #1903
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@merks merks merks approved these changes

Assignees

@wimjongman wimjongman

Labels
Enhancement Small change to improve the current supported functionality
Projects
None yet
Milestone
4.17
Development

Successfully merging this pull request may close these issues.

Use edge as default browser and use modern help
3 participants
@wimjongman @merks @speckyspooky