Mikeschinkel/refactor for CLI #5
Conversation
|
Mike, I can't review your PR. @HajagosNorbert, @judell, please review! |
|
I don't love the |
|
Also, I see it is using your version of the test server in its api definition. What's the status of that? If we were to merge this, would "xmlui run" work with it right now? |
- Rename `dbroot/` directory to `db/` throughout the project. - Update all file path references in `.xmlui/localsvr.json` to use new `db/` directory structure. - Move database file from `dbroot/data.db` to `db/data.db`. - Move all SQL query files from `dbroot/sql/` to `db/sql/`. - Add `data.db-*` pattern to `.gitignore` to exclude database backup files.
Good question. Different communities have different conventions, so I don't think we'll find any convention that feels right for everyone. I picked How to figure out what's "best?" Easiest way I can think to approximate best is to ask ChatGPT, and its results actually surprised me. After giving it all the background while trying not to bias it I recommended we use https://chatgpt.com/share/691cb1b1-fe40-800a-8264-a00493528492 BTW, I have already changed
Another good question. Currently this is all configurable in the configuration files, and getting the configuration working as a developer would expect it should work is what I have been finalizing the past few weeks. I chose to go with a two level configuration structure that has a user level configuration in So yes, it is all configurable. HOWEVER, might want to make it less configurable. I think there could be a lot of benefit to having a standardized directory structure. That has certainly worked well for WordPress as theme and plugin developers can just always assume the well-known directories are just there and so they don't have to write extra code to manage different directory. Certainly we can leave it configurable (and many other things are or can be left configurable; those things that need to be configurable) but I would vote to establish a standard directory structure and stick with it. A standard directory structure also makes writing docs easier and is IMO much easier for people to learn.
The current code, which I will be pushing soon after I submit this reply works with the code in the PR as of this morning. I would NOT merge this PR until we are all happy with the way the CLI works and we have updated all docs that explain to users how to get started using the CLI. As for the CLI being ready, I am at the point where I am ready to make changes to the repo so that all you'll need to do is clone it and build it, non of the complex stuff I asked you to do before that was so complex it failed. I know there are going to be bugs in the CLI — I have a hard time letting go of a release when I am not 100% certain it is fully tested and all potential bugs have been explored and fixed — but I think I need to let go of it and let everyone start to work with it and report bugs and/or requests change and/or request new features. Getting it so that I can give you all code you can easily compile and a release you can test is now my next task since I have gotten all the configuration work (or at least I think I have gotten it working.) |
I assume what you are asking about is answered by this? Note that that is functionally identical to what already exists in Jon's version at
The code that I have and am currently preparing in the release-candidate branch works with this PR to load the (There is a caveat and that is I don't have automated integration testing of the CLI yet and consequently every change I make to fix something could break one of the working use-cases, so when you try it, if it does not seem to work as I am claiming then it is almost certainly a quick-fix bug.) But again, let's not merge it yet; not until we are ready to release the CLI upon the world. My status? I am currently working to ensure you can just clone the CLI repo and run |
- Move all XMLUI application files from `webroot/` to `site/` directory structure. - Relocate resource files (`dashboard-mockup.png`, `demouser.png`, `favicon.ico`, `favicon.png`) from `webroot/resources/` to `site/resources/`. - Relocate JavaScript library files (`0.9.79.js`, `xmlui-0.9.81.js`) from `webroot/xmlui/` to `site/xmlui/`. - Add `.DS_Store` to `.gitignore` to prevent macOS metadata files from being tracked. This restructuring aligns the directory naming with common web application conventions where `site/` typically contains the deployable web assets.
- Directories were flattended at the project owner's request.
mikeschinkel
force-pushed
the
mikeschinkel/refactor-for-cli
branch
from
de204b5 to
5594cd6
Compare
|
Cool! Things are going in the right direction then, I like these changes. Sure, the client/test server stuff aren't separated as clearly to their own folders, but if the purpose is to make this into a demo project that just works with xmlui run, then I think it's worth making this tradeoff, so that the cli could have batter default values. |
|
Not sure if it was clear, but our project owner requested we flatten everything, to include all files we previously put in |
|
Yep, it was clear, and I think it is the right direction. |
- Add directory creation logic in `sqlite3.go` `Open()` method to ensure database directory exists before opening connection. - Update `go-dt` dependency from v0.2.1 to v0.2.4 across all modules (`cmd`, `test`, and `xmluisvr`). - Update `go-fsfix` dependency from v0.2.0 to v0.2.1 in `test` and `xmluisvr` modules. - Update indirect dependency `doublestar/v4` from v4.7.1 to v4.9.1. - Update `replace` directive path in `cmd/go.mod` to point to `../../localsvr/xmluisvr` (was `../xmluisvr`). - Refactor `cfgstore` constant names: replace `*ConfigDir` with `*ConfigDirType` suffix in `api_endpoint_v2.go`, `root_config_direct_load_test.go`, and `database.go`. - Move error constants `ErrConnectFailed`, `ErrInvalidConnectString`, `ErrFailedToPingDatabase`, `ErrFailedToOpenDatabase`, and `ErrFailedToExecuteQueries` from `dbpkg/errors.go` to `go-dt` package. - Update error references in `base_database.go`, `database_type.go`, `postgrespkg/postgres.go`, and `sqlite3pkg/sqlite3.go` to use `dt.*` package prefix instead of `dbpkg.*`. - Change default database root path from `"./db"` to `"."` in `common/const.go`. - Update test expectation in `root_config_v1_test.go` from `"./db/data.db"` to `"./data.db"`.
- Fix `Identifiers()` to use `append()` instead of direct indexing when filtering parameters, preventing sparse arrays that caused extraction failures. - Fix `DottedSelectors()` with same pattern to eliminate sparse arrays in selector extraction. - Document fix in `adrs/adr-002-infinite-loop-protection.md` with dated update entry explaining the root cause and solution.
3 participants
Here is a refactoring that supports the version of the CLI I have been working on.
Not expecting it to be merged until after we are comfortable with the CLI but it's here to review for now.