Move Non-C++ Files Around #1521
Conversation
|
LGTM on my Linux system (using Chinese Simplified locale)
|
|
Most new file locations look neat. But what is the reasoning behind the Move files in redist to data commit? These files are installed on freedesktop systems, so maybe they should remain in a common directory, separated from other files? |
I’m not very confident about it. According to the projects I’ve seen, they almost put these files into UPDATE: Considering GoldenDict is a relatively old project, I think it may be not necessary to move around the files in |
Yes, I think they should be separate. Especially the icon: with the move it becomes lost among the resource icons. |
OK ~, let me revert the commit later ~ |
|
@vedgy Done ~ |
|
How about actually dropping the commit, the reverting commit and force-pushing? Such back-and-forth negatively affects tracking files in git history. |
OK, wait a moment ~ |
WhiredPlanck
force-pushed
the
move-non-cpp-files
branch
from
c86f465
to
1aaa850
Compare
|
@vedgy Done ~ |
|
There are many occurrences of "resources.qrc" in GoldenDict's *.ui files. Could you please find relevant docs or other sources that explain why replacing some of these with "icons.qrc" should or should not be done? git grep -n resources.qrc |
Sorry for that I don’t find clear explains for this. But I checked the generated It seems that Qt will generate the "internal path"s according to the prefix at the head of the |
Good idea, thanks. The The *vcxproj* files are probably obsolete, but removing them in #1450 was not merged. If you cannot verify that the changes don't break GoldenDict on other platforms and the pull request doesn't fix actual bugs, I am afraid it is unlikely to be merged. |
Yep, as for now it’s a little difficult for me to verify it because my Visual Studio has some problems to run and I’m too tired to fix it. :/ But I will try to verify again.
Don’t worry now, since GoldenDict is a complex project and I realize it takes a lot of time to refactor some imperfect places with your suggestion last time. We can just wait for the right time. |
These files were added almost 10 years ago in ba94dd1 and haven't been maintained since 2013. The *.vcxproj* files are certainly out of date and not working now, because new source files have been added since 2013. One would need to put in a lot of effort to make the project and solution usable again. And whoever decides to do that would probably want to use a newer Visual Studio version. Simply removing this obsolete solution, then implementing a more modern one from scratch would likely be easier than updating. A benefit of removing these files is that new GoldenDict developers wouldn't have to waste their time wondering whether they should use goldendict.pro or these files. Also contributors wouldn't have to worry about updating the Visual Studio project while adding new source files or renaming/moving existing ones. For example, the unmaintained Visual Studio project hinders two pull requests: goldendict#1521 and goldendict#1532. @xiaoyifang proposed the same change in goldendict#1450. No one has explicitly rejected that older pull request. This commit reverts ba94dd1, except for the small change to goldendict.rc, which, judging by 281d3b7, is useful to the qmake-based Windows build.
WhiredPlanck
force-pushed
the
move-non-cpp-files
branch
from
1aaa850
to
90b1ec8
Compare
|
@vedgy Hi~ More than a year has passed, and recently this PR has come to my mind at last. I've made more refactors to the moving of those non-C++ files (basically the data files). I think you could take a look again now ~ |
|
Hi @WhiredPlanck. Can you please summarize your recent changes? (because the GitHub UI isn't very helpful here) I hope this PR can be simplified once #1639 is merged. #1532 conflicts with this PR, especially with the commit Linux-specific: Move desktop stuffs to data. #1532 is simpler and addresses a legacy-path issue, so I think it should be merged before this one. Furthermore, I just reread previous comments under this PR and found that moving files from redist to data has been reverted. Why restore this move now? |
Of course ~ Based on the changes before, the refactors also include the change of Visual Studio project and solution files since these files have not been removed yet.
And yes, this PR definitely can be simplified once #1639 is merged. Also I think it makes sense to move the desktop files and locale files to data, since they are really a kind of data.
That's great. I didn't notice #1532 before. It may also help simplify this PR ~
My fault. I just forgot that we had agreed on this before that not to move them. I was too impulsive to re-read the previous discussion before re-pushing the commits. 🥲 And just for reference: now I think we could move the desktop files into the Here is a project "crow-translate" for reference. It separates the app icons in a directory from other icons, and put the desktop files in the data directory. |
|
Splitting redist/ into data/redist/ and data/icons/redist/ should be OK, I think. But better wait till #1532 is merged to avoid conflicts. |
OK ~ |
These files were added almost 10 years ago in ba94dd1 and haven't been maintained since 2013. The *.vcxproj* files are certainly out of date and not working now, because new source files have been added since 2013. One would need to put in a lot of effort to make the project and solution usable again. And whoever decides to do that would probably want to use a newer Visual Studio version. Simply removing this obsolete solution, then implementing a more modern one from scratch would likely be easier than updating. A benefit of removing these files is that new GoldenDict developers wouldn't have to waste their time wondering whether they should use goldendict.pro or these files. Also contributors wouldn't have to worry about updating the Visual Studio project while adding new source files or renaming/moving existing ones. For example, the unmaintained Visual Studio project hinders two pull requests: #1521 and #1532. @xiaoyifang proposed the same change in #1450. No one has explicitly rejected that older pull request. This commit reverts ba94dd1, except for the small change to goldendict.rc, which, judging by 281d3b7, is useful to the qmake-based Windows build.
Overview
This PR is made up with some commits cherry-picked from #1515 , according to @vedgy 's advice, trying to move some non-C++ files around to make the source tree more tidy.
Details
See the commit message.