Those file permission constants are all over the place. If we want to use constants here, can't we put them globally into a central FS related file?

It is possible and @so-sahu had the same recommendation. I am personally more against it. Main reason are:

• We have to be careful with import into constants package becasue we cannot create loop in imports (it isn't big problem)
• It can create bad pattern design, where developers will put all constants into one place or we will end with weird mix what will affect using of code.
• Developers can use constants automatically without thinking, if they are using the best options. (Example: ignition file use different permissions as empty disk 0640 vs 0660)
• We have to be more careful with naming of constants, if they are exported.

I am not strongly against it but i prefer do it for small projects with few packages. Here i maybe keep current solution.

But if other people will have different opinion and can rewrite it, no problem.

Defining constants for file permissions might be a dangerous thing - but then I would say we stick to the octal numbers directly and don't use constants at all. That way you know that this operation is using 0640 for write a certain file.

Defining constants for file permissions might be a dangerous thing - but then I would say we stick to the octal numbers directly and don't use constants at all. That way you know that this operation is using 0640 for write a certain file.

More restricted linters can have problem with that, because they look at it as magic number and best practices for magic number is use variable and add some semantic with name for it. It can be simple solve with ignore specific error regexp.

But using constants have some benefits. Maybe good example how names adding semantic meaning in permFolder and permFile. folder needs execution for open of folder by default. file doesn't need that, but if file is binary you can create new constant permBinary and you will directly see why are you using this permission for file.

But i am okay with your suggestion but i want to hear more opinion. @so-sahu @lukasfrank @hardikdr

In my opinion, in a public project with permissions used in multiple places, it makes more sense to use constants with meaningful names. This approach offers several benefits:

1. Readability and Maintainability: Constants like permFolder and permFile make the code more understandable without needing to refer elsewhere.

2. Consistency: Using constants ensures consistent permission settings throughout the codebase, reducing the risk of errors.

3. Ease of Change: Constants allow for easy and centralized permission modifications if needed in the future.

4. Linting and Best Practices: Many linters and coding standards recommend using constants over magic numbers.

5. Semantic Meaning: Constants add semantic meaning, making it clear why specific permissions are set.

Adopting this approach will improve code readability, maintainability, and consistency.

Below is an in-depth analysis comparing centralizing file permission constants in one package with global environment variables v/s the current decentralized approach.

Suggestion 1: Centralizing File Permission Constants

• Pros:
  • Centralizing constants can improve maintainability by providing a single source of truth for file permission values.
  • It can enhance consistency in the naming and usage of constants across the codebase.
• Cons:
  • Importing constants from a central package can lead to circular dependencies if not managed carefully.
  • It may encourage developers to dump all constants into a single package, potentially leading to a bloated and less organized constants package.
  • Developers might use constants without considering the context, which could lead to incorrect permissions being used in certain scenarios.

Suggestion 2: Keep Constants Distributed

• Pros:
  • Keeping constants distributed allows each package to manage its own constants, promoting encapsulation and reducing the risk of circular dependencies.
  • It avoids creating a monolithic constants package, which can be harder to manage and maintain.
  • Developers are forced to think about the appropriate permissions for each specific use case, potentially leading to better code quality.
• Cons:
  • Naming conflicts might occur if different packages use similar constant names for different permissions.
  • Ensuring consistency in naming and usage across packages may require more manual effort.

In conclusion, for larger projects like libvirt-provider with multiple packages and potential for complex dependencies, keeping constants distributed can be a more practical approach. It promotes better encapsulation and allows each package to manage its own constants without relying heavily on a centralized package.