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

add codespell: configuration, workflow + get it to fix found typos #96

Merged
merged 7 commits into from
Oct 25, 2023

Conversation

yarikoptic
Copy link
Contributor

with the workflow you would get alerted when new typos introduced or found

@jmurugan-fzj
Copy link
Contributor

@yarikoptic Thank you so much for the workflow! Could you also please correct those three pending spell mistakes too, then I could approve and you could merge the changes 👍

@SteffenBrinckmann
Copy link
Contributor

Let's wait until thursday, if @yarikoptic does not find time, we can do those changes and merge

=== Do not change lines below ===
{
 "chain": [],
 "cmd": "codespell -w -i 3 -C 2",
 "exit": 0,
 "extra_inputs": [],
 "inputs": [],
 "outputs": [],
 "pwd": "."
}
^^^ Do not change lines above ^^^
=== Do not change lines below ===
{
 "chain": [],
 "cmd": "codespell -w -i 3 -C 2 ./tests/app_tests/component_tests/test_ontology_configuration_extended.py",
 "exit": 0,
 "extra_inputs": [],
 "inputs": [],
 "outputs": [],
 "pwd": "."
}
^^^ Do not change lines above ^^^
=== Do not change lines below ===
{
 "chain": [],
 "cmd": "codespell -w",
 "exit": 0,
 "extra_inputs": [],
 "inputs": [],
 "outputs": [],
 "pwd": "."
}
^^^ Do not change lines above ^^^
@yarikoptic
Copy link
Contributor Author

sorry, missed initial request. rebased, redone fixing... something odd -- codespell managed to not save the fix, should report (some time)

❯ codespell -w -i 3 -C 2
:     We are aware of different run-times in different operating system. A backend test has different execution times on Windows (Thinkpad E495: 57.9sec), macOS (Macbook Air 2020: 20.8sec) and Linux (Thinkpad E495: 14.8sec). The graphical user interface seems to work similarly fast on all operating systems. We will investigate how to speed up the backend for Windows users in the future.
: 
> *Why dont' you package everything in a flatpak or snap?*
:     Containerization is a great concept for many software as it separates the host operating system from the software. This concept is not for PASTA-ELN because we require and want to extend extractors [See ...](extractors.md). As such the user/scientist should be able to change code and require libraries that are not included by default. These libraries cannot be added to the container. Scientist developed extractors cannot be archived in a container which should be self-sufficient and encapsulated. (Also, one could think of creating detours via system-calls / demons on the host system. However, flatpak and snap severely restrict host system-calls and cannot interact with processes on the host system.)
: 
*Why dont' you package everything in a flatpak or snap?*
	dont' ==> don't (Y/n) Y
FIXED: ./docs/source/faqs.rst
❯ git diff
❯ 

so I did it manually... should be green now:

❯ codespell
❯ codespell --version
2.2.6

@jmurugan-fzj
Copy link
Contributor

sorry, missed initial request. rebased, redone fixing... something odd -- codespell managed to not save the fix, should report (some time)

❯ codespell -w -i 3 -C 2
:     We are aware of different run-times in different operating system. A backend test has different execution times on Windows (Thinkpad E495: 57.9sec), macOS (Macbook Air 2020: 20.8sec) and Linux (Thinkpad E495: 14.8sec). The graphical user interface seems to work similarly fast on all operating systems. We will investigate how to speed up the backend for Windows users in the future.
: 
> *Why dont' you package everything in a flatpak or snap?*
:     Containerization is a great concept for many software as it separates the host operating system from the software. This concept is not for PASTA-ELN because we require and want to extend extractors [See ...](extractors.md). As such the user/scientist should be able to change code and require libraries that are not included by default. These libraries cannot be added to the container. Scientist developed extractors cannot be archived in a container which should be self-sufficient and encapsulated. (Also, one could think of creating detours via system-calls / demons on the host system. However, flatpak and snap severely restrict host system-calls and cannot interact with processes on the host system.)
: 
*Why dont' you package everything in a flatpak or snap?*
	dont' ==> don't (Y/n) Y
FIXED: ./docs/source/faqs.rst
❯ git diff
❯ 

so I did it manually... should be green now:

❯ codespell
❯ codespell --version
2.2.6

@yarikoptic Thank you so much for the quick response, could you please go ahead and merge the changes too, would be helpful too 👍

@SteffenBrinckmann
Copy link
Contributor

@jmurugan-fzj : According to:
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/merging-a-pull-request
only people with a "push access" can merge, hence @yarikoptic cannot. To keep with the one-person-reviews and one-person-merges approach, I will merge, since @jmurugan-fzj reviewed.

@yarikoptic
Copy link
Contributor Author

FWIW confirming that I can't... and yeah -- no need to give me super powers ;)

@SteffenBrinckmann SteffenBrinckmann merged commit c7d42b6 into PASTA-ELN:main Oct 25, 2023
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.

3 participants