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

web: Allow changing file system location #1141

Merged
merged 6 commits into from
Apr 12, 2024

Conversation

joseivanlopez
Copy link
Contributor

@joseivanlopez joseivanlopez commented Apr 9, 2024

By default, all the file systems are created as partitions at the disk selected for installation. This PR adds an optiton for changing the location of a file system, allowing to select a target disk or to follow the installation device. Moreover, it offers an option to create a dedicated LVM volume group. For more details see https://github.com/openSUSE/agama/blob/master/doc/storage_ui.md#file-systems.

Bonus: This PR introduces quite some changes to enable type checking for several components.

Screenshots

localhost_8080_ (37)

@coveralls
Copy link

coveralls commented Apr 9, 2024

Coverage Status

coverage: 74.657% (-0.01%) from 74.671%
when pulling 3b4ec55 on joseivanlopez:change-location
into 67081db on openSUSE:master.

@joseivanlopez joseivanlopez force-pushed the change-location branch 3 times, most recently from 7a3ddde to 225b82e Compare April 10, 2024 11:29
</div>
</fieldset>

<fieldset className="stack">
Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't tested it yet, but looks like the (Firefox) problem described at #354 has been solved (see https://bugzilla.mozilla.org/show_bug.cgi?id=1829874)

So, If it's the case, we could disable the fieldset instead of going field by field. Should work natively.

@joseivanlopez joseivanlopez force-pushed the change-location branch 2 times, most recently from c701cce to a7a833e Compare April 11, 2024 12:47
@joseivanlopez joseivanlopez marked this pull request as ready for review April 11, 2024 15:11
Copy link
Contributor

@dgdavid dgdavid left a comment

Choose a reason for hiding this comment

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

Thanks @joseivanlopez!

Code wise it looks good to me. I have, however, not blocking doubts I'd like to discuss in a short meeting. But please, go ahead and merge it is nobody opposes.

BTW, excuse me for not noticing in the sync calls you were adding type checking in files that will collide (a lot?) with #1138 . And big thank you for joining to the adventure of enabliy the type checking. I really believe it will be a good thing for Agama.

web/src/client/storage.js Show resolved Hide resolved
/**
* @todo Enable type checking and ensure the components are called with the correct props.
*
* @note The default value for `settings` should be `undefined` instead of an empty object, and
Copy link
Contributor

Choose a reason for hiding this comment

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

NP: if so, we should keep the "?." operator.n

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now it cannot be undefined. If we initialize settings to undefined, then I would change the rest of the code a bit instead of having .? everywhere.

web/src/components/storage/ProposalPage.test.jsx Outdated Show resolved Hide resolved
@joseivanlopez joseivanlopez merged commit 766a871 into agama-project:master Apr 12, 2024
1 check passed
@imobachgs imobachgs mentioned this pull request May 17, 2024
imobachgs added a commit that referenced this pull request May 17, 2024
Prepare for releasing Agama 8. It includes the following pull requests:

* #884
* #886
* #914
* #918
* #956
* #957
* #958
* #959
* #960
* #961
* #962
* #963
* #964
* #965
* #966
* #969
* #970
* #976
* #977
* #978
* #979
* #980
* #981
* #983
* #984
* #985
* #986
* #988
* #991
* #992
* #995
* #996
* #997
* #999
* #1003
* #1004
* #1006
* #1007
* #1008
* #1009
* #1010
* #1011
* #1012
* #1014
* #1015
* #1016
* #1017
* #1020
* #1022
* #1023
* #1024
* #1025
* #1027
* #1028
* #1029
* #1030
* #1031
* #1032
* #1033
* #1034
* #1035
* #1036
* #1038
* #1039
* #1041
* #1042
* #1043
* #1045
* #1046
* #1047
* #1048
* #1052
* #1054
* #1056
* #1057
* #1060
* #1061
* #1062
* #1063
* #1064
* #1066
* #1067
* #1068
* #1069
* #1071
* #1072
* #1073
* #1074
* #1075
* #1079
* #1080
* #1081
* #1082
* #1085
* #1086
* #1087
* #1088
* #1089
* #1090
* #1091
* #1092
* #1093
* #1094
* #1095
* #1096
* #1097
* #1098
* #1099
* #1100
* #1102
* #1103
* #1104
* #1105
* #1106
* #1109
* #1110
* #1111
* #1112
* #1114
* #1116
* #1117
* #1118
* #1119
* #1120
* #1121
* #1122
* #1123
* #1125
* #1126
* #1127
* #1128
* #1129
* #1130
* #1131
* #1132
* #1133
* #1134
* #1135
* #1136
* #1138
* #1139
* #1140
* #1141
* #1142
* #1143
* #1144
* #1145
* #1146
* #1147
* #1148
* #1149
* #1151
* #1152
* #1153
* #1154
* #1155
* #1156
* #1157
* #1158
* #1160
* #1161
* #1162
* #1163
* #1164
* #1165
* #1166
* #1167
* #1168
* #1169
* #1170
* #1171
* #1172
* #1173
* #1174
* #1175
* #1177
* #1178
* #1180
* #1181
* #1182
* #1183
* #1184
* #1185
* #1187
* #1188
* #1189
* #1190
* #1191
* #1192
* #1193
* #1194
* #1195
* #1196
* #1198
* #1199
* #1200
* #1201
* #1203
* #1204
* #1205
* #1206
* #1207
* #1208
* #1209
* #1210
* #1211
* #1212
* #1213
* #1214
* #1215
* #1216
* #1217
* #1219
* #1220
* #1221
* #1222
* #1223
* #1224
* #1225
* #1226
* #1227
* #1229
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.

3 participants