Skip to content

Conversation

@evgeni
Copy link
Member

@evgeni evgeni commented Sep 2, 2025

No description provided.

@evgeni
Copy link
Member Author

evgeni commented Sep 2, 2025

Hmm, this would break

BuildRequires: (npm(babel-preset-env) >= 1.6.0 with npm(babel-preset-env) < 2.0.0)

@nadjaheitmann why does ACD depend on preset-env directly?

@nadjaheitmann
Copy link

Hmm, this would break

BuildRequires: (npm(babel-preset-env) >= 1.6.0 with npm(babel-preset-env) < 2.0.0)

@nadjaheitmann why does ACD depend on preset-env directly?

Looks like remains of the original webpack setup. Should I change it to babel/core instead?

@nadjaheitmann
Copy link

@evgeni How comes you're bumping from version 1.7.x to 7.9.5? :)

@evgeni
Copy link
Member Author

evgeni commented Sep 8, 2025

Well, it also moves from the babel-preset-env package to @babel/preset-env (but the pkg name remains, due to the way we handle group-packages). So no idea why the jump is so big, but I think it's rather expected.

And yes, I think you can drop babel-preset-env in favor of @babel/core (or do plugins not need that at all? cc @MariaAga )

@MariaAga
Copy link
Member

MariaAga commented Sep 8, 2025

theforeman/foreman#10319
uses:

   "@babel/core": "~7.25.2",
    "@babel/preset-env": "~7.25.2",
    "@babel/preset-react": "~7.25.2"

If that is the question? @evgeni

@evgeni
Copy link
Member Author

evgeni commented Sep 8, 2025

@MariaAga yeah, the question is: do plugins need to depend on that themself or not? given the above PR is not yet merged, I assume "they do now, but soon won't need to"

@MariaAga
Copy link
Member

MariaAga commented Sep 8, 2025

https://github.com/theforeman/foreman_discovery/blob/develop/babel.config.js
They use the foreman builder, and have babel setups sometimes

@nadjaheitmann
Copy link

You lost me here. What do I need to do about the babel setup once the Foreman PR is merged? Are the dependencies automatically pulled in by Foreman?

If you are planning to merge the Foreman PR soon, then I tend to wait until it is merged and then update the foreman_acd dependencies (and save multiple rounds of packaging and updates). Unless you prefer to fix it to have green pipelines or whatever is blocking at the moment @evgeni .

@MariaAga
Copy link
Member

MariaAga commented Sep 9, 2025

It doesnt look like its (the foreman pr to remove foreman/build) is planned to be merged this month

@nadjaheitmann
Copy link

OK, thanks for the info, then I'll just update ACD to have the same dependencies as any other plugin.

@nadjaheitmann
Copy link

@evgeni Updating ACD to properly work with the newest foreman packages requires quite some effort as we would need to refactor the tests that are using an outdated and unsupported testing framework. So feel free to go ahead with this PR even if it breaks ACD. We will decide what to do with this, eventually.

@evgeni
Copy link
Member Author

evgeni commented Sep 12, 2025

That would mean to remove acd from nightly, tho. As our pipelines do not allow packages with broken dependencies.
(It might work, as it's only a build-dependency, but still risky)

@nadjaheitmann
Copy link

That would mean to remove acd from nightly, tho. As our pipelines do not allow packages with broken dependencies. (It might work, as it's only a build-dependency, but still risky)

Ok, we'll clarify that.

@nadjaheitmann
Copy link

That would mean to remove acd from nightly, tho. As our pipelines do not allow packages with broken dependencies. (It might work, as it's only a build-dependency, but still risky)

@evgeni Feel free to remove ACD from foreman-packaging if this unblocks the pipeline for this PR :)

@nadjaheitmann
Copy link

@evgeni Feel free to remove ACD from foreman-packaging if this unblocks the pipeline for this PR :)

Talking about removing ACD: #12677 That would unblock this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants