Ecosyste.ms: Timeline

Browse the timeline of events for every public repo on GitHub. Data updated hourly from GH Archive.

kirkrodrigues

kirkrodrigues created a comment on an issue on y-scope/clp-ffi-js
> @kirkrodrigues / @davemarco > > Any objection if we [`npm unpublish`](https://docs.npmjs.com/cli/v9/commands/npm-unpublish) 0.3.1 and 0.3.2 (see #48) on the npm registry? No, go for it.

View on GitHub

kirkrodrigues created a review on a pull request on y-scope/clp-ffi-js

View on GitHub

kirkrodrigues created a review on a pull request on y-scope/clp-ffi-js

View on GitHub

kirkrodrigues created a review on a pull request on y-scope/clp-ffi-js
One optional comment otherwise lgtm. For the PR title, how about: ``` ci(publish-to-npm): Download and extract build as single artifact at the correct path (fixes #48). ```

View on GitHub

kirkrodrigues created a review comment on a pull request on y-scope/clp-ffi-js
Yep

View on GitHub

kirkrodrigues created a review on a pull request on y-scope/clp-ffi-js

View on GitHub

kirkrodrigues created a review comment on a pull request on y-scope/clp-ffi-js
This is optional, but do you want to call it something more readable like `clp-ffi-js` or `clp-ffi-js-dist` so that it's obvious when downloaded?

View on GitHub

kirkrodrigues created a review on a pull request on y-scope/clp-ffi-js
One optional comment otherwise lgtm. For the PR title, how about: ``` ci(publish-to-npm): Download and extract build as single artifact at the correct path (fixes #48). ```

View on GitHub

kirkrodrigues created a review on a pull request on y-scope/clp-ffi-js
One optional comment otherwise lgtm. For the PR title, how about: ``` ci(publish-to-npm): Download and extract build as single artifact at the correct path (fixes #48). ```

View on GitHub

kirkrodrigues created a review on a pull request on y-scope/clp-ffi-js

View on GitHub

kirkrodrigues created a review on a pull request on y-scope/clp-ffi-js
For the PR title, how about: ``` build(cmake): Extract CMake project name and version from package.json. ``` It's subjective, but I think `build` is a more appropriate type since this chang...

View on GitHub

kirkrodrigues created a review comment on a pull request on y-scope/clp-ffi-js
> but yeah why don't we directly change the metadata in package.json and let cmake sync from it? lol Yep. > with that said, realistically, in this project, those variables are not getting ref...

View on GitHub

kirkrodrigues created a review on a pull request on y-scope/clp-ffi-js

View on GitHub

kirkrodrigues created a review comment on a pull request on y-scope/clp-ffi-js
```suggestion if("${PACKAGE_JSON_CONTENT}" MATCHES "\"version\":[ ]*\"([^\"]+)\"") ```

View on GitHub

kirkrodrigues created a review comment on a pull request on y-scope/clp-ffi-js
```suggestion "${CMAKE_MATCH_1}" ```

View on GitHub

kirkrodrigues created a review comment on a pull request on y-scope/clp-ffi-js
```suggestion "${CMAKE_MATCH_1}" ```

View on GitHub

kirkrodrigues created a review comment on a pull request on y-scope/clp-ffi-js
Why do we need/want to make these cache variables?

View on GitHub

kirkrodrigues created a review comment on a pull request on y-scope/clp-ffi-js
```suggestion if("${PACKAGE_JSON_CONTENT}" MATCHES "\"name\":[ ]*\"([^\"]+)\"") ``` To avoid [double-variable expansion](https://cmake.org/cmake/help/latest/command/if.html#variable-expansion).

View on GitHub

kirkrodrigues created a review comment on a pull request on y-scope/clp-ffi-js
How about `CLP_FFI_JS_PACKAGE_JSON_CONTENT`? (To follow our convention of prefixing local variables with the project name.)

View on GitHub

kirkrodrigues created a review on a pull request on y-scope/clp-ffi-js

View on GitHub

kirkrodrigues created a review on a pull request on y-scope/yscope-dev-utils
For the PR title, how about: ``` Update the clang-format config for clang-format v19. ```

View on GitHub

kirkrodrigues created a comment on a pull request on y-scope/clp
> Alternatively, shall we look into turning this into a Vite-React app? That way we don't have to manage the build configs on our own with webpack. We could, I guess.

View on GitHub

kirkrodrigues created a review comment on a pull request on y-scope/clp
Yeah, that's my understanding based on this: https://webpack.js.org/configuration/configuration-types/#exporting-a-function I think the old code was wrong since it should've been using the `env`...

View on GitHub

kirkrodrigues created a review on a pull request on y-scope/clp

View on GitHub

kirkrodrigues created a review comment on a pull request on y-scope/yscope-dev-utils
```suggestion MainIncludeChar: "Quote" MaxEmptyLinesToKeep: 1 ``` Another new option omitted from the release notes.

View on GitHub

kirkrodrigues created a review comment on a pull request on y-scope/yscope-dev-utils
```suggestion BreakConstructorInitializers: "BeforeColon" BreakFunctionDefinitionParameters: false ``` This is a new option that's unfortunately omitted from the release notes.

View on GitHub

kirkrodrigues created a review comment on a pull request on y-scope/yscope-dev-utils
This is an option for Java code (rather than C++), right?

View on GitHub

kirkrodrigues created a review on a pull request on y-scope/yscope-dev-utils

View on GitHub

kirkrodrigues created a review on a pull request on y-scope/clp-ffi-js

View on GitHub

kirkrodrigues created a review comment on a pull request on y-scope/clp-ffi-js
Right, good idea.

View on GitHub

Load more