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

Removed infile features #647

Merged
merged 2 commits into from
Sep 1, 2020
Merged

Removed infile features #647

merged 2 commits into from
Sep 1, 2020

Conversation

lebensterben
Copy link
Contributor

There are multiple issues regarding infile support.

Notably,

  • It doesn't support some basic syntax of BitBake.
  • It doesn't fully comply to BitBake's conventions.
  • The differences from BitBake is not well-documented.

Since this is never used, I propose to remove this functionality entirely.

#645
#646

@phmccarty
Copy link
Contributor

@lebensterben I am okay with removing these features, but I would prefer not to see all of the code style changes interleaved in the same commit... Did your editor make the style changes automatically?

If you would like to make some changes to code style, make a separate commit (with rationale in the commit description if necessary), and make sure that the changes apply to the entire code base. Or you can omit the style changes from this PR if you prefer.

@lebensterben
Copy link
Contributor Author

lebensterben commented Sep 1, 2020

Yeah. emacs reformatted some files according and reordered imports.

@lebensterben
Copy link
Contributor Author

It's done now.
BTW, would you add a .editorconfig or .flake8 file to the root, so that modern editors can use your per-project style settings?

@phmccarty
Copy link
Contributor

phmccarty commented Sep 1, 2020

The flake8 config is already enforced by Github Actions. You can run the checks locally by running make check. The behavior is controlled via the toplevel setup.cfg.

I like the idea of adding a .editorconfig, since it can cover basic style issues. Opened an issue for that: #648

@phmccarty
Copy link
Contributor

@lebensterben It looks like all test files under tests/testfiles/bb/ can be removed as well? Can you check that?

Otherwise, changes look good to me.

@lebensterben
Copy link
Contributor Author

@phmccarty I removed them (tests/testfiles/bb) in the new commit.

Copy link
Contributor

@phmccarty phmccarty left a comment

Choose a reason for hiding this comment

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

+1, thanks!

@phmccarty phmccarty merged commit e8ee8e3 into clearlinux:master Sep 1, 2020
@lebensterben lebensterben deleted the remove-infile-feature branch September 2, 2020 00:04
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