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

config : Remove IE options #6233

Merged
merged 1 commit into from
Jan 29, 2025

Conversation

ivanimanishi
Copy link
Member

@ivanimanishi ivanimanishi commented Jan 27, 2025

This PR removes the IE-specific config files, as they are now maintained in a separate internal repository.

Note that I don't think there is any need to update the Changes file, since there is not relevant code updates in gaffer itself, but let me know if you think otherwise.

Checklist

  • I have read the contribution guidelines.
  • I have updated the documentation, if applicable.
  • I have tested my change(s) in the test suite, and added new test cases where necessary.
  • My code follows the Gaffer project's prevailing coding style and conventions.

@johnhaddon
Copy link
Member

Now using "black" style (in use at IE)

I'm not sure how I feel about this. I know this file is only for use by IE, but it's in the Gaffer repo, so I kindof think it should use the Gaffer conventions. Maybe you'd prefer to host it outside the repo anyway?

@ivanimanishi
Copy link
Member Author

Now using "black" style (in use at IE)

I'm not sure how I feel about this. I know this file is only for use by IE, but it's in the Gaffer repo, so I kindof think it should use the Gaffer conventions. Maybe you'd prefer to host it outside the repo anyway?

We are no longer reading those files directly from the gaffer repository when building gaffer at IE. We have them in a separate internal repository, which needs to match our coded style. My understanding is that the only reason now to keep it in gaffer itself is as an example for other about how to potentially run a custom install of gaffer.

I'm fine with either removing them completely from the gaffer public repo or updating them to their current IE state (this PR), black style and all (I don't want to go through the trouble of reformatting our internal options files to match gaffer's style).

What do you prefer?

@johnhaddon
Copy link
Member

Oh, cool. If you don't need the file then let's just remove it. The CI setup still provides some examples on how to configure a build, and is probably a bit easier to follow.

@ivanimanishi ivanimanishi changed the title ie/options : Multiple updates to support Gaffer 1.4+ and new code style config : Remove IE options Jan 28, 2025
@ivanimanishi
Copy link
Member Author

Updated the PR, as discussed, to remove the config/ie files.

@johnhaddon johnhaddon merged commit b8a5e44 into GafferHQ:1.5_maintenance Jan 29, 2025
7 checks passed
@johnhaddon
Copy link
Member

Thanks Ivan!

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.

2 participants