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

SYMBIAN: Simplify access to file server session. #3157

Merged
merged 1 commit into from Jul 16, 2021

Conversation

@fedor4ever
Copy link
Contributor

@fedor4ever fedor4ever commented Jul 15, 2021

This fixes bug #12728 too.

@digitall
Copy link
Member

@digitall digitall commented Jul 16, 2021

Since this PR only changes code associated with the SymbianOS port and @fedor4ever is the SymbianOS porter, then I think this is fine to merge as-is.

@digitall digitall merged commit 6ddfb08 into scummvm:master Jul 16, 2021
7 of 8 checks passed
7 of 8 checks passed
@github-actions
Windows (win32, x86-windows, x86, --enable-faad --enable-mpeg2 --enable-discord --disable-fribidi...
Details
@github-actions
Windows (x64, x64, x64-windows, --enable-faad --enable-mpeg2 --enable-discord --disable-fribidi, ...
Details
@github-actions
Windows (arm64, arm64, arm64-windows, --enable-faad --enable-mpeg2 --enable-discord --disable-fri...
Details
@github-actions
Xcode (macosx, -scheme ScummVM-macOS, --disable-nasm --enable-faad --enable-mpeg2, a52dec faad2 f...
Details
@github-actions
Xcode (ios7, -scheme ScummVM-iOS CODE_SIGN_IDENTITY="" CODE_SIGNING_ALLOWED=NO, --disable-nasm --...
Details
@github-actions
Ubuntu (ubuntu-latest, sdl2-config, --enable-c++11, libsdl2-dev libsdl2-net-dev liba52-dev libjpe...
Details
@github-actions
Ubuntu (ubuntu-18.04, sdl-config, --disable-c++11, libsdl1.2-dev libsdl-net1.2-dev liba52-dev lib... Ubuntu (ubuntu-18.04, sdl-config, --disable-c++11, libsdl1.2-dev libsdl-net1.2-dev liba52-dev lib...
Details
@codacy-production
Codacy Static Code Analysis Codacy Static Code Analysis
Details
@sev-
Copy link
Member

@sev- sev- commented Jul 16, 2021

Well, the code is not conformant to our code formatting guidelines. It was not a great idea to merge it as is :/

@digitall
Copy link
Member

@digitall digitall commented Jul 16, 2021

@sev- : Sorry about that. I can't spot any major deviation from the code formatting guidelines here, at least compared to the code prior to the change. What part of https://wiki.scummvm.org/index.php?title=Code_Formatting_Conventions ?

@sev-
Copy link
Member

@sev- sev- commented Jul 16, 2021

@digitall For example, RFs& fs = FsSession(); instead of RFs &fs = FsSession(); and couple of other instances of the similar formatting

@digitall
Copy link
Member

@digitall digitall commented Jul 17, 2021

@sev- : OK. Fixed in 4cbbabf , though that incorrect formatting was present prior to this PR i.e. f0afa0c#diff-8666146c0bb6b2539e251a8d7ce9dc3fcac6101db56eef94e58c44b982e1673fR129

I think we could do with adding a github Action or similar to do an automatic style check on PRs against astyle or similar?

@fedor4ever
Copy link
Contributor Author

@fedor4ever fedor4ever commented Jul 17, 2021

@digitall , @sev- , It's good to automate style checking.

@fedor4ever fedor4ever deleted the fedor4ever:symbian_fix_crash branch Jul 17, 2021
digitall added a commit to digitall/scummvm that referenced this pull request Jul 19, 2021
Correcting the code format according to the guidelines.
- Line 48 (const HardwareInput& x, const HardwareInput& y) is replaced by (const HardwareInput &x, const HardwareInput &y).
- Line 57 (const HardwareInput& x) is replaced by (const HardwareInput &x).

Example of another case: scummvm#3157
huhungahuru added a commit to huhungahuru/scummvm that referenced this pull request Jul 19, 2021
Line 118: Fixed (TDes& aDataFolder) code by (TDes &aDataFolder).
Reference: scummvm#3157 (comment)
huhungahuru added a commit to huhungahuru/scummvm that referenced this pull request Jul 19, 2021
Line 46: Fixed (TDes& aDataFolder) code by (TDes &aDataFolder).
Reference: scummvm#3157 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants