Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Refactoring: Purge code generators of IFileSystemAccess2. #938

Merged
merged 1 commit into from
Feb 8, 2022
Merged

Conversation

petervdonovan
Copy link
Collaborator

@petervdonovan petervdonovan commented Feb 4, 2022

#929 is not the first bug that was related to IFileSystemAccess2 (see acf9859). In #929, writing to a file caused an error in org.eclipse.core.runtime.SubMonitor, a progress reporting class which I could not find the source code for. It is annoying for trivial operations like writing to files to cause IllegalStateExceptions in seemingly unrelated parts of the framework whose state is only ever touched in other, distant parts of our code base.

For this reason, this PR eliminates IFileSystemAccess2 wherever possible. In #149, @cmnrd gave a valid reason why we should use the IFileSystemAccess2 interface (it specifies the src-gen directory), but this PR preserves the code that gets the location of the src-gen directory from the IFileSystemAccess2.

EDIT: This change has been tested in Epoch by compiling at least one test program in Epoch for each target.

@petervdonovan petervdonovan force-pushed the purge-fsa branch 2 times, most recently from 3d492a8 to e13a722 Compare February 5, 2022 21:48
@petervdonovan petervdonovan marked this pull request as ready for review February 5, 2022 23:25
Copy link
Collaborator

@cmnrd cmnrd left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for the clean up :)

Any ideas why the CI tests did not run and how to trigger them on this PR?

@petervdonovan petervdonovan changed the base branch from master to schedulers2 February 7, 2022 16:47
@petervdonovan petervdonovan changed the base branch from schedulers2 to master February 7, 2022 16:47
@petervdonovan
Copy link
Collaborator Author

No, I do not know why integration tests are not running. I have heard that switching the base branch and switching back can force some things to update on GitHub, but that did not work here. The CI workflow isn't written to be manually triggered either, though I wonder if it should be. In order to trigger CI, I may have to close this PR and open a new one...

@cmnrd
Copy link
Collaborator

cmnrd commented Feb 8, 2022

Looks like that did the trick!

@cmnrd cmnrd merged commit 5095540 into master Feb 8, 2022
@cmnrd cmnrd deleted the purge-fsa branch February 8, 2022 15:35
@lhstrh lhstrh added the refactoring Code quality enhancement label Mar 17, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
refactoring Code quality enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants