Code Quality: Clean up Files.Actions phase 1

[0x5bfa]

Code Quality: Clean up Files.Actions

Motivation & Context

• Improved readability

Warning
Please do not review code lines that I did not changed. I cannot make more diff. If there're more codes that are preferred to change, I will do that and mark that PR as the final one for that stage.

PR Checklist

• Were these changes approved in an issue or discussion with the project maintainers? In order to prevent extra work, feature requests and changes to the codebase must be approved before the pull request will be reviewed. This prevents extra work for the contributors and maintainers.
  Related Feature: Improving Performance, optimizations, reliability and codebase  #4180
• Did you build the app and test your changes?
• Did you check for accessibility? You can use Accessibility Insights for this.
• Did you remove any strings from the en-us resource file?
  • Did you search the solution to see if the string is still being used?
• Did you implement any design changes to an existing feature?
  • Was this change approved?
• Are there any other steps that were used to validate these changes?

Screenshots

None

[ferrariofilippo]

You should consider opening a different PR to refactor the initialization of context in most actions.
There are some little things that might be improved, but apart from that looks good

There are three missing ;:

• DecompressArchive.cs at line 36
• DecompressArchiveHere.cs at line 30
• BaseSetAsAction.cs at line 32

[0x5bfa]

You should consider opening a different PR to refactor the initialization of context in most actions.

How come should I? I think it’s fine so far. Probably I will split up phase 1 PR into multiple PRs, so reviewers can review quickly and easily.

[ferrariofilippo]

How come should I? I think it’s fine so far.

There are a lot of Actions which use
private readonly IContentApplicationContext context = Ioc.Default.GetRequired<IContentPageContext>();

I think the assignment should be moved to the constructor

[0x5bfa]

I think the assignment should be moved to the constructor

All of them were moved.

[0x5bfa]

Ready for final review.

[ferrariofilippo]

There might be errors with tasks not awaited

[ferrariofilippo]

Some more style-related issues, but I don't think they are blockers

[ferrariofilippo]

LGTM

[0x5bfa]

@yaira2 Ready to merge?

[yaira2]

@0x5bfa this looks like a big change, I'd like to get a second review before merging.

[0x5bfa]

Should I have split up into like 3 PRs? I can do that instead of fixing conflicts and waiting reviews.

[yaira2]

We can do them together

[yaira2]

@0x5bfa besides for formatting, were there any other changes?

[0x5bfa]

Change overview

• Moved initializers to the constructors (i.e. dependency injection initializings)
• Improved formatting
• Updated return values of Execute(); (i.e. removing async for non-async) You can see this changing in ferrariofilippo's reviews