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.

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

docs, ctest break, serverMain, e2e, clang version format-check update #520

Closed
wants to merge 2 commits into from

Conversation

joaomarques90
Copy link
Contributor

@joaomarques90 joaomarques90 commented Dec 8, 2021

native docs --> update and improvement of the doc && Flag '-a' in shortopts (ServerMain.cpp.120) but not addressed, causing it to break. Presented all flags options.

ctest --> comment truncate exception (MmapVectorImpl.h.34) [previously questioned if it did matter]. Previously 6 tests failed.

serverMain --> server arguments valid verification (options_value > 0) && new value for threads: max_threads_concurrency ("-j -1")

e2e --> upgrade for paths contaning spaces

format-check --> update for clang-13

Local Native Run/Tested on Ubuntu 20.04.3 LTS via WSL2

native docs --> update and improvement of the doc &&  Flag '-a' in shortopts (ServerMain.cpp.120)  but not addressed, causing it to break. Presented all flags options.

ctest --> comment truncate exception (MmapVectorImpl.h.34) [previously questioned if it did matter]. Previously 6 tests failed.

serverMain --> server arguments valid verification (options_value > 0) && new value for threads: max_threads_concurrency ("-j -1")

e2e --> upgrade for paths contaning spaces

Local Native Run/Tested on Ubuntu 20.04.3 LTS via WSL2
@hannahbast
Copy link
Member

@joaomarques90 Thank you for this PR. You opened a second PR (522) which is very similar. You can (and should) update this PR (by pushing to the underlying branch).

format-check --> fix  & update for clang-13
@joaomarques90 joaomarques90 changed the title docs, ctest break, server, e2e docs, ctest break, serverMain, e2e, clang version format-check update Dec 10, 2021
@joka921
Copy link
Member

joka921 commented Jan 20, 2022

Can you please split this up into a PR that only does the necessary changes to make it work in your setup
(the quotation marks in the bash script, and the Removal of the "truncate" command in MmapVectorImpl.h .

The reworking of ServerMain.cpp is done in another PR by me (we integrate an alternative to getopt) making your (reasonable) suggestions obsolete.

The native-setup.md is completely outdated, and we have to think about either rewriting it or throwing it out, so this is also not relevant for now.

The change to clang-format-13 also should be (if we want to do this) a different PR, since it requires reformatting of the
complete codebase. Why did you suggest this?

@joaomarques90
Copy link
Contributor Author

joaomarques90 commented Jan 24, 2022

The change to clang-format-13 also should be (if we want to do this) a different PR, since it requires reformatting of the
complete codebase. Why did you suggest this?

Sorry for the late response, I just saw it now.
I have suggested that clang-format version because I have seen this:

- name: Install clang 13

Therefore, it made me believe (since it is in cmake workflow) that the format-check.sh should contain that version as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants