Skip to content

add a ci step for Json_Diagnostic_Positions - #4579

Merged
nlohmann merged 6 commits into
nlohmann:developfrom
hnampally:develop
Jan 3, 2025
Merged

add a ci step for Json_Diagnostic_Positions#4579
nlohmann merged 6 commits into
nlohmann:developfrom
hnampally:develop

Conversation

@hnampally

@hnampally hnampally commented Jan 1, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

This PR brings the change to add a CI step for the new feature Json_Diagnostic_Positions

Pull request checklist

Read the Contribution Guidelines for detailed information.

  • Changes are described in the pull request, or an existing issue is referenced.
  • The test suite compiles and runs without error.
  • Code coverage is 100%. Test cases can be added by editing the test suite.
  • The source code is amalgamated; that is, after making changes to the sources in the include/nlohmann directory, run make amalgamate to create the single-header files single_include/nlohmann/json.hpp and single_include/nlohmann/json_fwd.hpp. The whole process is described here.

@hnampally
hnampally requested a review from nlohmann as a code owner January 1, 2025 01:25
@coveralls

coveralls commented Jan 1, 2025 •
edited
Loading

Copy link
Copy Markdown

Coverage Status

coverage: 99.639%. remained the same
when pulling c1e8021 on hnampally:develop
into 4f64d8d on nlohmann:develop.

gregmarr
gregmarr reviewed Jan 1, 2025
View reviewed changes
Comment thread cmake/ci.cmake Resolved
Comment thread cmake/ci.cmake Resolved
nlohmann
nlohmann requested changes Jan 1, 2025
View reviewed changes

@nlohmann nlohmann left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Please sign-off your commit(s).

Comment thread cmake/ci.cmake Resolved
@hnampally

hnampally commented Jan 1, 2025

Copy link
Copy Markdown
Contributor Author

Please sign-off your commit(s).

I made the changes and and pushed this commit 2d04d40 from the browser, so I think it was not properly signed off, sorry about that.

@nlohmann

nlohmann commented Jan 1, 2025

Copy link
Copy Markdown
Owner

Please sign-off your commit(s).

I made the changes and and pushed this commit 2d04d40 from the browser, so I think it was not properly signed off, sorry about that.

Do you have access to https://github.com/nlohmann/json/pull/4579/checks?check_run_id=35047024834? This contains an explanation how to fix this.

@hnampally

hnampally commented Jan 1, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Please sign-off your commit(s).

I made the changes and and pushed this commit 2d04d40 from the browser, so I think it was not properly signed off, sorry about that.

Do you have access to https://github.com/nlohmann/json/pull/4579/checks?check_run_id=35047024834? This contains an explanation how to fix this.

Yes, thanks for the pointer, I will fix and push it shortly.

@hnampally
add a ci step for Json_Diagnostic_Positions
b615508 
Signed-off-by: Harinath Nampally <harinath922@gmail.com>
@hnampally
Update ci.cmake to address review comments
39434ce 
Signed-off-by: Harinath Nampally <harinath922@gmail.com>
@hnampally
address review comment
c78941a 
Signed-off-by: Harinath Nampally <harinath922@gmail.com>
nlohmann
nlohmann requested changes Jan 1, 2025
View reviewed changes
Comment thread cmake/ci.cmake Resolved
@hnampally
fix typo in the comment
66de11c 
Signed-off-by: Harinath Nampally <harinath922@gmail.com>
gregmarr
gregmarr reviewed Jan 2, 2025
View reviewed changes
Comment thread cmake/ci.cmake Resolved
@hnampally
fix typos in ci.cmake
ffbc778 
Signed-off-by: Harinath Nampally <harinath922@gmail.com>
nlohmann
nlohmann requested changes Jan 2, 2025
View reviewed changes
Comment thread cmake/ci.cmake Resolved
nlohmann
nlohmann requested changes Jan 2, 2025
View reviewed changes
Comment thread cmake/ci.cmake Resolved
@hnampally
invoke the new ci step from ubuntu.yml
c1e8021 
Signed-off-by: Harinath Nampally <harinath922@gmail.com>
@github-actions github-actions Bot added the CI label Jan 3, 2025
nlohmann
nlohmann approved these changes Jan 3, 2025
View reviewed changes

@nlohmann nlohmann left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Looks good to me.

@nlohmann nlohmann added this to the Release 3.11.4 milestone Jan 3, 2025
@nlohmann
nlohmann merged commit a6255cc into nlohmann:develop Jan 3, 2025
@nlohmann nlohmann mentioned this pull request Jan 3, 2025
Merged
4 tasks done
@nlohmann

nlohmann commented Jan 3, 2025

Copy link
Copy Markdown
Owner

Thanks!

slowriot pushed a commit to slowriot/json that referenced this pull request Jan 10, 2025
@hnampally @slowriot
add a ci step for Json_Diagnostic_Positions (nlohmann#4579)
5eebff1 
* add a ci step for Json_Diagnostic_Positions

Signed-off-by: Harinath Nampally <harinath922@gmail.com>

* Update ci.cmake to address review comments

Signed-off-by: Harinath Nampally <harinath922@gmail.com>

* address review comment

Signed-off-by: Harinath Nampally <harinath922@gmail.com>

* fix typo in the comment

Signed-off-by: Harinath Nampally <harinath922@gmail.com>

* fix typos in ci.cmake

Signed-off-by: Harinath Nampally <harinath922@gmail.com>

* invoke the new ci step from ubuntu.yml

Signed-off-by: Harinath Nampally <harinath922@gmail.com>

---------

Signed-off-by: Harinath Nampally <harinath922@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nlohmann nlohmann nlohmann approved these changes
+1 more reviewer
@gregmarr gregmarr gregmarr left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CI CMake M

Projects

None yet

Milestone

Release 3.12.0

Development

Successfully merging this pull request may close these issues.

4 participants

@hnampally @coveralls @nlohmann @gregmarr
⌂ 换网址